classification
Title: base64_codec uses assert for runtime validity checks
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: alex.henderson, berker.peksag, ezio.melotti, lemburg, martin.panter, ncoghlan, vstinner
Priority: normal Keywords: patch

Created on 2013-04-25 07:40 by ncoghlan, last changed 2019-07-29 11:43 by vstinner.

Files
File name Uploaded Description Edit
issue17840.patch alex.henderson, 2013-07-06 12:49 Removes error-mode assertions from encodings/*_codec.py review
issue17840.patch alex.henderson, 2013-07-12 16:02 New patch raising ValueError review
Messages (11)
msg187762 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-04-25 07:40
encodings.base64_codec currently uses "assert errors=='strict'" in a few places, since it doesn't actually support any of the Unicode specific error handling modes.

This should either be discarded entirely (and document that the error handling mode is irrelevant for this codec), or else turned into a real check that raises ValueError if an unsupported error mode is passed in.

I have a slight preference for just ignoring the error mode as irrelevant (since this isn't a text encoding in the normal Unicode serialisation-as-bytes sense).
msg192434 - (view) Author: Alex Henderson (alex.henderson) * Date: 2013-07-06 12:49
I see that there is identical usage of "assert errors=='strict'" in a number of similar encodings modules:

base64_codec.py
bz2_codec.py
hex_codec.py
quopri_codec.py
uu_codec.py
zlib_codec.py

The error handling mode is irrelevant for all these codecs, so the attached patch addresses them all (choosing to ignore the error mode and documenting this).
msg192561 - (view) Author: Alex Henderson (alex.henderson) * Date: 2013-07-07 14:31
Having discussed this with Ezio, I think the better option might be to raise ValueError instead - if someone is expecting to be able to silently recover from errors they won't be able to, and should find out about this sooner rather than later.
I'll upload an updated patch shortly.
msg192638 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2013-07-08 11:55
On 07.07.2013 16:31, Alex Henderson wrote:
> Having discussed this with Ezio, I think the better option might be to raise ValueError instead - if someone is expecting to be able to silently recover from errors they won't be able to, and should find out about this sooner rather than later.
> I'll upload an updated patch shortly.

+1
msg192710 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-07-09 02:22
ValueError works for me.
msg192951 - (view) Author: Alex Henderson (alex.henderson) * Date: 2013-07-12 16:02
OK, now raises ValueError on passing anything other than 'strict'.

Note that for the incremental classes I've put checking in __init__ so that ValueError is raised when non-'strict' values are passed to the constructor, not when the incremental encode/decode methods are subsequently called.
msg234303 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-01-19 06:19
Is this patch likely to go ahead? It has been sitting around a while and would conflict with patches I am working on.

If so, I reckon it would be good to factor out some of the new bits of code (_check_strict, _StrictErrors) into a common place, like the “codecs” module.
msg234306 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-01-19 07:24
Thanks for the patch. I left a couple of comments on Rietveld.
msg234307 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-01-19 07:35
Would also be good to document that errors='ignored' is not allowed. Currently the documentation says

The following string values are defined and implemented by all standard Python codecs:

* 'strict'
* 'ignore'
msg234613 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-24 13:06
I'd be slightly more inclined to file the fact that "ignore" isn't supported by the base64 codec as a potential bug, with changing the docs as one possible resolution.

It shouldn't hold up switching from the assertions to proper runtime checks.
msg348628 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-29 11:43
This issue is 6 years old and has patches: it is no newcomer friendly, I remove the "easy" keyword.
History
Date User Action Args
2019-07-29 11:43:52vstinnersetkeywords: - easy
nosy: + vstinner
messages: + msg348628

2015-03-21 18:03:22serhiy.storchakasetstage: patch review -> needs patch
2015-01-24 13:06:02ncoghlansetmessages: + msg234613
2015-01-19 07:35:52martin.pantersetmessages: + msg234307
2015-01-19 07:24:06berker.peksagsetmessages: + msg234306
2015-01-19 07:16:19berker.peksagsetnosy: + berker.peksag
stage: test needed -> patch review

versions: + Python 3.5, - Python 3.4
2015-01-19 06:19:21martin.pantersetnosy: + martin.panter
messages: + msg234303
2013-07-12 16:02:07alex.hendersonsetfiles: + issue17840.patch

messages: + msg192951
2013-07-09 02:22:50ncoghlansetmessages: + msg192710
2013-07-08 11:55:50lemburgsetnosy: + lemburg
messages: + msg192638
2013-07-07 14:31:02alex.hendersonsetmessages: + msg192561
2013-07-06 12:49:55alex.hendersonsetfiles: + issue17840.patch

nosy: + alex.henderson
messages: + msg192434

keywords: + patch
2013-04-27 16:58:47ezio.melottisetkeywords: + easy
nosy: + ezio.melotti
2013-04-25 07:40:50ncoghlancreate