Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 19, 2026

@serhiy-storchaka
Copy link
Member Author

This is a draft because I am going to apply the error handling changes in a separate issue first.

We can also apply some optimizations -- cache the ignored chars and returning on fast path after ignored characters.

@serhiy-storchaka serhiy-storchaka force-pushed the base64-decode-ignorechars branch from 1b0bb87 to 192d535 Compare January 19, 2026 11:19
@serhiy-storchaka serhiy-storchaka marked this pull request as ready for review January 19, 2026 11:20
@serhiy-storchaka
Copy link
Member Author

Well, the issue with error handling was just that the error message could be more specific in some cases. It is not a bug. It can be included in this PR.

@serhiy-storchaka serhiy-storchaka force-pushed the base64-decode-ignorechars branch from 192d535 to 4f3847e Compare January 19, 2026 11:23
@moneebullah25
Copy link
Contributor

No worries, that was the point I was just free on Sunday, so I push out a PR in case you haven't started on it. BTW your PR looks good, I am no maintainer but you just have to run ./python Tools/build/generate_global_objects.py to generate the updated header files. I hope it helps.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm fine with changing validate default to True if ignorechars is specified. The change produces more precise error messages. It also adds a lot of tests which is always a good thing :-)

@serhiy-storchaka serhiy-storchaka merged commit 7febbe6 into python:main Jan 26, 2026
51 checks passed
@serhiy-storchaka serhiy-storchaka deleted the base64-decode-ignorechars branch January 26, 2026 18:11
@bedevere-bot

This comment was marked as off-topic.

@zware

This comment was marked as off-topic.

@gpshead
Copy link
Member

gpshead commented Jan 26, 2026

Taking a quick look: The docs in this PR changed the default for validate and strict_mode from False to True. But versionchanged does not mention that change and there was no deprecation period. Did this behavior change? The docs need fixing at a minimum but if the behavior changed we also need a deprecation cycle. What I understand @vstinner above to be saying is to change the default only in the case of the new parameter being supplied?

@gpshead
Copy link
Member

gpshead commented Jan 26, 2026

that may just be a simple doc fix. the declaration lines need to not state a value for the parameters who's default behavior varies based on context:

.. function:: b64decode(s, altchars=None, validate=False)
              b64decode(s, altchars=None, validate=True, *, ignorechars)
.. function:: a2b_base64(string, /, *, strict_mode=False)
              a2b_base64(string, /, *, strict_mode=True, ignorechars)

The text below describes a dynamic behavior on both, but people will absolutely misread the function signature and not read the text if we do not indicate it in the signature.

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.

7 participants