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

Modified the trustymail code to handle SPF redirects #29

Merged
merged 2 commits into from
Nov 27, 2017

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Nov 22, 2017

This looks like a bigger change than it is because I:

  • Alphabetized the imports at the top of the file
  • Broke the spf_scan() function up into three separate functions

This will resolve #7.

@jsf9k jsf9k requested a review from h-m-f-t November 22, 2017 19:54
domain.syntax_errors.append(error.msg)


def get_spf_record_text(resolver, domain_name, domain, follow_redirect=False):
Copy link
Member

Choose a reason for hiding this comment

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

Should follow_redirect default to True?

Copy link
Member Author

@jsf9k jsf9k Nov 22, 2017

Choose a reason for hiding this comment

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

See my comments below, in the spf_scan function.

"""
# If an SPF record exists, record the raw SPF record text in the
# Domain object
record_text_not_following_redirect = get_spf_record_text(resolver, domain.domain_name, domain)
Copy link
Member Author

Choose a reason for hiding this comment

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

get_spf_record_text is called with follow_redirect=False here because we want the SPF record that directly corresponds to domain.domain_name. We stash this value inside of domain.spf on line 291.

if record_text_not_following_redirect:
domain.spf.append(record_text_not_following_redirect)

record_text_following_redirect = get_spf_record_text(resolver, domain.domain_name, domain, True)
Copy link
Member Author

Choose a reason for hiding this comment

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

get_spf_record_text is called with follow_redirect=True here because we want to follow the redirects until we get an SPF record that we can use to determine the expected behavior when we run spf.check() inside of the check_spf_record function on line 198.

@h-m-f-t h-m-f-t merged commit 06caaa4 into master Nov 27, 2017
@h-m-f-t h-m-f-t deleted the bugfix/spf_redirect branch November 27, 2017 16:22
@h-m-f-t
Copy link
Member

h-m-f-t commented Nov 27, 2017

👍 🇺🇸

mcdonnnj pushed a commit that referenced this pull request Jan 23, 2023
…m-forked-repos

Make workflow run when a PR is opened, synchronized, or reopened
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants