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

Alphanumeric enforcement on HeaderName breaks Implementation with Memphis. #1030

Closed
turulix opened this issue Jul 16, 2023 · 3 comments · Fixed by #1031 or #1032
Closed

Alphanumeric enforcement on HeaderName breaks Implementation with Memphis. #1030

turulix opened this issue Jul 16, 2023 · 3 comments · Fixed by #1031 or #1032
Assignees
Labels
bug Confirmed reproducible bug

Comments

@turulix
Copy link

turulix commented Jul 16, 2023

As stated above the Alphanumeric enforcement introduced in 0.30.0 breaks custom headers used by memphis.

if s.contains(|c: char| !c.is_ascii_alphanumeric() && c != '-') {
return Err(ParseHeaderNameError);
}

Memphis uses Nats in the background and uses SpecialHeaders like $memphis_producedBy which inclue $, _ and therefor triggering the ParseHeaderNameError

It also appears that the enforcement dosn't happen while inserting into the HeaderMap but rather when reading it at

headers.append(HeaderName::from_str(key).unwrap(), value.trim().to_string());

Causing the entire library to panic rather than returning an error.

But I don't understand why the headers are being restricted in the first place, as something like the headers above work absolutly fine, and I didn't find anything in the NATS documentation restricting the use of headers like that. I think if a user wants to use funky HeaderNames just let them do it.

@caspervonb caspervonb self-assigned this Jul 17, 2023
@caspervonb caspervonb added bug Confirmed reproducible bug and removed needs triage labels Jul 17, 2023
@caspervonb caspervonb changed the title Alphanumeric enforcement on Headernames breaks Implementation with Memphis. Alphanumeric enforcement on HeaderName breaks Implementation with Memphis. Jul 17, 2023
@caspervonb
Copy link
Collaborator

But I don't understand why the headers are being restricted in the first place, as something like the headers above work absolutly fine, and I didn't find anything in the NATS documentation restricting the use of headers like that. I think if a user wants to use funky HeaderNames just let them do it.

Header names are specced to be ASCII 33 to 126, excluding colon so that's why.

@turulix
Copy link
Author

turulix commented Jul 17, 2023

Ah okay, didn't know. Makes sence. Thank you for this fix! :)

@caspervonb
Copy link
Collaborator

Thanks for the catch and report! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed reproducible bug
Projects
None yet
2 participants