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

Fix a error handling in UUID parser #167

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

Drvi
Copy link
Collaborator

@Drvi Drvi commented May 16, 2023

We were checking pos <= len at the wrong place for UUIDs which have an insufficient length which lead to a bounds error when we were trying to skip over the malformed bytes. In tests we now also actually shrink the source, not just the len argument, which covers this error.

Fixes #166

@Drvi Drvi requested a review from nickrobinson251 May 16, 2023 10:32
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.18 🎉

Comparison is base (b879c62) 87.39% compared to head (c520244) 87.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
+ Coverage   87.39%   87.58%   +0.18%     
==========================================
  Files          10       10              
  Lines        1856     1860       +4     
==========================================
+ Hits         1622     1629       +7     
+ Misses        234      231       -3     
Impacted Files Coverage Δ
src/hexadecimal.jl 98.59% <100.00%> (+0.04%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Drvi Drvi requested a review from quinnj May 16, 2023 11:30
@@ -176,9 +177,12 @@ end
@noinline function _backtrack_error(source, pos, len, code, segment_len, hi, lo, pl)
pos -= segment_len # Backtrack to the start of the invalid hex
fastseek!(source, pos - 1)
while _HEX_LUT[peekbyte(source, pos) + 0x01] != 0xFFFF
b = peekbyte(source, pos)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we check pos is valid here?

maybe a couple lines up add an @assert 1 <= pos <= len(is that right?)

Copy link
Collaborator Author

@Drvi Drvi May 16, 2023

Choose a reason for hiding this comment

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

pos should always be valid here -- we only use _backtrack_error once we check the length would fit a UUID value and then we iterate in fixed sized steps (segment_len).

@Drvi Drvi merged commit 7b04eaa into main May 16, 2023
12 checks passed
@Drvi Drvi deleted the td-fix-uuid-error-handling branch May 16, 2023 12:30
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.

BoundsError for test that parses incorrect UUID
2 participants