Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes issue 132 (Public Suffix List (PSL) Updates Too Often) #133

Merged
merged 9 commits into from
Sep 12, 2023

Conversation

Matthew-Grayson
Copy link
Contributor

@Matthew-Grayson Matthew-Grayson commented May 14, 2023

🗣 Description

Added code to set the PSL file's last modified time to reflect time of last download.

💭 Motivation and context

PSL should only be downloaded once per day. The current code attempts to enforce that by only downloading a new PSL, when the local copy is older than 24 hours:

psl_age = datetime.now() - datetime.fromtimestamp(
    stat(PublicSuffixListFilename).st_mtime
)
if psl_age > timedelta(hours=24):
    updatePSL(PublicSuffixListFilename)

stat.st_mtime checks the last modified date of the local PSL, however, the last modified date will often be 2-4 days ago even if the PSL was downloaded minutes before.

Resolves #132.

🧪 Testing

I ran Trustymail against an array of domains.
The first domain was tested and then the code waited for a local PSL to be created before testing the remaining domains. I logged all STDOUT to see if the "PSL Updated" message appeared more than once.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Finalize version.

✅ Post-merge checklist

  • Create a release.

Add code to set modified time to now for newly downloaded PSL file to avoid multiple downloads per day.
@jsf9k jsf9k self-assigned this May 15, 2023
@jsf9k jsf9k added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use version bump This issue or pull request increments the version number python Pull requests that update Python code labels May 15, 2023
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm requesting one very small change.

src/trustymail/domain.py Outdated Show resolved Hide resolved
@jsf9k jsf9k requested review from jasonodoom and a team May 15, 2023 14:42
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

cisagovbot pushed a commit that referenced this pull request Jul 12, 2023
@jsf9k
Copy link
Member

jsf9k commented Sep 12, 2023

I also did some testing with these changes and verified that they function as expected.

@jsf9k jsf9k merged commit 08430e2 into cisagov:develop Sep 12, 2023
22 checks passed
cisagovbot pushed a commit that referenced this pull request Jul 30, 2024
⚠️ CONFLICT! Lineage pull request for: skeleton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use python Pull requests that update Python code version bump This issue or pull request increments the version number
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

Public Suffix List (PSL) Updates Too Often
3 participants