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

Allow automatic extraction of lid from List-Id header #241

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sbp
Copy link
Contributor

@sbp sbp commented Jun 15, 2022

This PR means that you can run python3 dkim_id.py example.eml - and the lid is automatically extracted from the List-Id header, if present, without incurring the bug in #113.

@sbp sbp force-pushed the automatic-lid branch from c8f2b9c to 177a4f8 Compare June 15, 2022 21:01
@sebbASF
Copy link
Contributor

sebbASF commented Jun 15, 2022

However fixing the bug will effectively change the generate ids.

@sbp
Copy link
Contributor Author

sbp commented Jun 15, 2022

This doesn't fix the bug. The bug is still present, unless the new automatic argument is set to True. The intended use is for parsing messages from the command line.

Copy link
Contributor

@sebbASF sebbASF left a comment

Choose a reason for hiding this comment

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

Also I think there needs to be a test to show that the generated id is the same whether automatic mode is used or the lid is provided

elif len(argv) == 3: # add lid
lid: Optional[bytes] = argv[2].encode("utf-8")
automatic: bool = False
if lid == b"-":
Copy link
Contributor

Choose a reason for hiding this comment

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

This special value needs documenting

@@ -302,6 +302,7 @@ def rfc6376_reformed_canon(
head_canon: bool = False,
body_canon: bool = False,
lid: Optional[bytes] = None,
automatic: bool = False,
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs documenting

data: bytes, lid: Optional[bytes] = None
data: bytes,
lid: Optional[bytes] = None,
automatic: bool = False,
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs documenting

@@ -381,7 +392,9 @@ def unpibble32(text: str) -> bytes:
return base64.b32decode(encoded.translate(table))


def dkim_id(data: bytes, lid: Optional[bytes] = None) -> str:
def dkim_id(
data: bytes, lid: Optional[bytes] = None, automatic: bool = False
Copy link
Contributor

Choose a reason for hiding this comment

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

automatic arg needs documenting

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