10000 Fix Msg.Len calculation when an RR contains a padded base64 string by brianshea2 · Pull Request #1652 · miekg/dns · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix Msg.Len calculation when an RR contains a padded base64 string #1652

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Msg.Len calculation when an RR contains a padded base64 string #1652

wants to merge 1 commit into from

Conversation

brianshea2
Copy link
Contributor

The RR.len functions use base64.StdEncoding.DecodedLen to obtain the length of a base64 string field.

Per base64.StdEncoding.DecodedLen's documentation:

DecodedLen returns the maximum length in bytes of the decoded data

Unfortunately, that function does not account for padding characters.

This PR replaces the use of that function with one that calculates the exact length of the decoded data, and adds some tests to prove that.

@brianshea2 brianshea2 requested review from miekg and tmthrgd as code owners May 30, 2025 07:01
@miekg
Copy link
Owner
miekg commented May 30, 2025

So in Go's base64 decoding, the decodedLen disregards padding, but when decoding it is not? Isn't that a bug in Go?

@brianshea2
Copy link
Contributor Author

So in Go's base64 decoding, the decodedLen disregards padding, but when decoding it is not? Isn't that a bug in Go?

When decoding into bytes, the padding is discarded as normal. The DecodedLen function is meant as a quick estimate (notice it says "maximum" in the docs), mainly used to size a buffer to decode the bytes in to. It does not account for padding, which would decrease the actual decoded length, I believe simply as a short cut. The new function introduced in this PR to get the exact decoded length is negligibly more expensive, but necessary to get the exact length so things like padding a message to a specific block length can work.

@miekg
Copy link
Owner
miekg commented May 31, 2025

ok, fair, but Len() should be used as an indication on how large the buffer should be, if this makes it slightly larger, no harm is done. So I thing carrying all the new (test) code is not worth it

@brianshea2
Copy link
Contributor Author

ok, fair, but Len() should be used as an indication on how large the buffer should be, if this makes it slightly larger, no harm is done. So I thing carrying all the new (test) code is not worth it

Msg.Len can have more use than only an indication on how large a buffer to hold the packed msg should be. I came across this while attempting to get the current size of a response to calculate the amount of edns0 padding would be needed to grow the response to a certain size. In that case, the current inaccuracy of Msg.Len causes harm to that calculation, not adding enough padding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miekg miekg Awaiting requested review from miekg miekg is a code owner

@tmthrgd tmthrgd Awaiting requested review from tmthrgd tmthrgd is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brianshea2 @miekg
0