classification
Title: struct.pack_into check boundary error message ignores offset
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: andrewnester, louielu, serhiy.storchaka
Priority: normal Keywords:

Created on 2017-02-25 13:53 by louielu, last changed 2017-04-04 10:47 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 424 merged andrewnester, 2017-03-03 12:41
Messages (11)
msg288564 - (view) Author: Louie Lu (louielu) * Date: 2017-02-25 13:53
For this situation, check boundary error message didn't correctly show out.

>>> import struct
>>> import ctypes
>>> byte_list = ctypes.create_string_buffer(1)
>>> struct.pack_into('b', byte_list, 5, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: pack_into requires a buffer of at least 1 bytes

Since offset is setting at 5, it should at least need `offset + soself->s_size` bytes to store it.
msg288882 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-03 12:41
Thanks! Just added PR fixing this.
msg288884 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-03 13:41
I'm not sure the new error message is better. It is not known what is wrong -- the size of the buffer, or the offset?

I think that the better error message should include three numbers: the offset, the size of the destination buffer, and the size of packed data or the minimal size of the destination buffer.
msg288885 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-03 13:44
yeah, I also thought about this too.
Something like 'pack_into requires a buffer of at least 6 bytes (size is 1, offset is 5)'
msg288886 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-03 13:56
Or "pack_into requires a buffer of at least 6 bytes for packing 1 bytes at offset 5".
msg288887 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-03 14:01
thanks Serhiy! just implemented your variant in my PR.
msg288978 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-04 13:55
Different error messages are needed if original offset < 0. For example packing 4 bytes with offset -2 always fails, not depending of the size of the buffer. Packing into buffer of size 10 with offset -11 always fails, not depending of the size of packed data.

struct.pack_into('<I', bytearray(10), -2, 123)
struct.pack_into('<B', bytearray(10), -11, 123)
msg289021 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-05 16:33
Thanks Serhiy! Just implemented new error messages in my PR for case you mentioned.
msg290081 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-24 11:27
any updates on this issue? looks like PR is ready to go
msg291119 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-04 10:46
New changeset f78b119364b521307237a1484ba5f43f42300898 by Serhiy Storchaka (Andrew Nester) in branch 'master':
bpo-29649: Improve struct.pack_into() boundary error messages (#424)
https://github.com/python/cpython/commit/f78b119364b521307237a1484ba5f43f42300898
msg291120 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-04 10:47
Thank you for your contribution Andrew.
History
Date User Action Args
2017-04-04 10:47:37serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg291120

stage: patch review -> resolved
2017-04-04 10:46:28serhiy.storchakasetmessages: + msg291119
2017-03-24 11:27:30andrewnestersetmessages: + msg290081
2017-03-05 16:33:21andrewnestersetmessages: + msg289021
2017-03-04 13:55:49serhiy.storchakasetassignee: serhiy.storchaka
messages: + msg288978
2017-03-03 21:18:25terry.reedysettitle: struct.pack_into check boundary error message didn't respect offset -> struct.pack_into check boundary error message ignores offset
type: enhancement
stage: patch review
2017-03-03 14:01:13andrewnestersetmessages: + msg288887
2017-03-03 13:56:47serhiy.storchakasetmessages: + msg288886
2017-03-03 13:44:35andrewnestersetmessages: + msg288885
2017-03-03 13:41:45serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg288884
2017-03-03 12:41:53andrewnestersetnosy: + andrewnester
messages: + msg288882
2017-03-03 12:41:28andrewnestersetpull_requests: + pull_request354
2017-02-25 13:53:04louielucreate