Skip to content

Conversation

xoac
Copy link

@xoac xoac commented Feb 17, 2022

close #519

The documentation say that valid ASCII is between (32..127) char. But function is_visible_ascii() check also for \t char what is coded as 9.

@lilyball
Copy link

Your PR description doesn't match the code. is_visible_ascii() actually allows for \t (even though it's not visible; I think the name here is wrong, it should be called is_valid_ascii instead). And what your PR does is restrict from_str to ASCII, which has nothing to do with \t.

@xoac
Copy link
Author

xoac commented Feb 24, 2022

I just matched implementation of from_static() that has exactly the same description for ASCII requirements as from_str and use the same function (is_visible_ascii) to check correctness but panic instead of returning error.

So the code should be correct if from_static() is correct.

@lilyball
Copy link

lilyball commented Mar 6, 2022

Note: I put a number of comments in the linked issue (#519).

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.

HeaderValue::from_str does not validate that the string is valid ascii

2 participants