Add readahead buffer to C decoder by andreer · Pull Request #268 · agronholm/cbor2

andreer · 2025-12-18T16:40:36Z

Changes

Add readahead buffer to C decoder to improve performance by avoiding many small reads.

I've not added test or updated docs yet, will do later if this makes sense.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

You've added tests (in tests/) which would fail without your patch
You've updated the documentation (in docs/), in case of behavior changes or new
features
You've added a new changelog entry (in docs/versionhistory.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version.
If, say, your patch fixes issue #123, the entry should look like this:

- Fix big bad boo-boo in the encoder
  (`#123 <https://github.com/agronholm/cbor2/issues/123>`_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

coveralls · 2025-12-18T16:45:08Z

coverage: 94.58%. remained the same
when pulling 24f0714 on andreer:master
into 0bcf400 on agronholm:master.

agronholm · 2025-12-18T18:38:12Z

I think a changelog entry should be added for this.

agronholm

Looks good, just one change request.

source/decoder.c

andreer · 2025-12-21T09:50:25Z

On further inspection I'm also introducing another bug here in the case where decode_from_bytes is called on tag data during decoding, as described in the documentation:

cbor2/docs/customizing.rst

Lines 99 to 112 in 403c2ce

    
               def tag_hook(decoder, tag, shareable_index=None): 
        
                   # Return all other tags as-is 
        
                   if tag.tag != 3000: 
        
                       return tag 
        
                   # Create a raw instance before initializing its state to make it possible for cyclic 
        
                   # references to work 
        
                   instance = MyType.__new__(MyType) 
        
                   decoder.set_shareable(shareable_index, instance) 
        
                   # Separately decode the state of the new object and then apply it 
        
                   state = decoder.decode_from_bytes(tag.value) 
        
                   instance.__dict__.update(state) 
        
                   return instance

Currently I believe this will overwrite the readahead buffer, losing bytes. Will need another look.

agronholm · 2025-12-26T20:56:52Z

Is it good for another round now?

andreer · 2025-12-26T20:58:37Z

Ugh, a little bit of shotgun coding there. Since that failed twice I'd like to review it again more thoroughly tomorrow before adding a changelog entry.

- configurable readahead buffer (default 4KB) to reduce Python read() calls - add read_size parameter to CBORDecoder.__init__ - per-instance buffers for thread-safety

agronholm · 2025-12-27T15:03:44Z

Please don't force push changes. It makes it hard for me to track changes you've made.

andreer · 2025-12-27T15:04:29Z

Ok, now I've convinced myself I understand what the issue was and that it's properly fixed. I ended up refactoring a bit, there was a lot of back and forth in the commits so I thought it made most sense to squash it. The diff should be smaller and hopefully easier to review now.

agronholm · 2025-12-27T15:35:10Z

Now I'll have to re-read everything, as I have no clue what the problem was before or what you did to fix it.

andreer · 2025-12-27T20:57:02Z

Ah, sorry about that. The un-squashed commit history is here: https://github.com/andreer/cbor2/commits/unsquashed/

The issue I referred to was a use-after-free which was fixed in andreer@559d387

agronholm · 2025-12-27T23:56:16Z

Thanks. I'll give this a new review today (Sunday).

agronholm

I can't find any problems. Looks good! Thanks.

andreer · 2025-12-31T19:34:57Z

Thank you, and happy new year!

Pastea · 2026-01-07T14:29:28Z

Hi, this PR was linked to the CVE-2025-68131 but I think that this is incorrect, could you please confirm if instead the correct fixing commit is this one f1d701cd2c411ee40bb1fe383afe7f365f35abf0? Thanks

agronholm · 2026-01-07T14:39:03Z

Hi, this PR was linked to the CVE-2025-68131 but I think that this is incorrect, could you please confirm if instead the correct fixing commit is this one f1d701cd2c411ee40bb1fe383afe7f365f35abf0? Thanks

Indeed. I don't know why this PR was linked to that CVE, or how to control what PR is linked if at all.

Pastea · 2026-01-07T15:06:16Z

I think that it takes data from the references inside the github advisory, just created a PR to adjust it.

Backport the patch[1] which fixes this vulnerability as mentioned in the comment[2]. Details: https://nvd.nist.gov/vuln/detail/CVE-2025-68131 [1] agronholm/cbor2@f1d701c [2] agronholm/cbor2#268 (comment) Dropped changes to the changelog from the original commit. Signed-off-by: Ankur Tyagi <ankur.tyagi85@gmail.com> Signed-off-by: Anuj Mittal <anuj.mittal@oss.qualcomm.com>

Source: meta-openembedded MR: 307157, 302820 Type: Security Fix Disposition: Merged from meta-openembedded ChangeID: daacf50 Description: Backport the patch[1] which fixes this vulnerability as mentioned in the comment[2]. Details: https://nvd.nist.gov/vuln/detail/CVE-2025-68131 [1] agronholm/cbor2@f1d701c [2] agronholm/cbor2#268 (comment) Dropped changes to the changelog from the original commit. Signed-off-by: Ankur Tyagi <ankur.tyagi85@gmail.com> Signed-off-by: Anuj Mittal <anuj.mittal@oss.qualcomm.com> Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>

agronholm requested changes Dec 20, 2025

View reviewed changes

source/decoder.c Outdated Show resolved Hide resolved

add readahead buffer to C decoder

4756c20

- configurable readahead buffer (default 4KB) to reduce Python read() calls - add read_size parameter to CBORDecoder.__init__ - per-instance buffers for thread-safety

andreer force-pushed the master branch from 559d387 to 4756c20 Compare December 27, 2025 15:01

Merge branch 'master' into master

24f0714

agronholm approved these changes Dec 29, 2025

View reviewed changes

agronholm merged commit fb4ee16 into agronholm:master Dec 29, 2025
14 checks passed

agronholm added a commit that referenced this pull request Dec 30, 2025

Updated the version history entry for #268

22acea4

Pastea mentioned this pull request Jan 7, 2026

[GHSA-wcj4-jw5j-44wh] CBORDecoder reuse can leak shareable values across decode calls github/advisory-database#6625

Merged

Uh oh!

Conversation

andreer commented Dec 18, 2025 • edited by agronholm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Checklist

Updating the changelog

Uh oh!

coveralls commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agronholm commented Dec 18, 2025

Uh oh!

agronholm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreer commented Dec 21, 2025

Uh oh!

agronholm commented Dec 26, 2025

Uh oh!

andreer commented Dec 26, 2025

Uh oh!

agronholm commented Dec 27, 2025

Uh oh!

andreer commented Dec 27, 2025

Uh oh!

agronholm commented Dec 27, 2025

Uh oh!

andreer commented Dec 27, 2025

Uh oh!

agronholm commented Dec 27, 2025

Uh oh!

agronholm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreer commented Dec 31, 2025

Uh oh!

Pastea commented Jan 7, 2026

Uh oh!

agronholm commented Jan 7, 2026

Uh oh!

Pastea commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreer commented Dec 18, 2025 •

edited by agronholm

Loading

coveralls commented Dec 18, 2025 •

edited

Loading