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

Key Headers HashMap with SendStr instead of &'static str #14

Merged
merged 1 commit into from Sep 8, 2014

Conversation

reem
Copy link
Contributor

@reem reem commented Sep 7, 2014

This fixes an unsafe transmute that could cause use-after-frees
on collision resolution in the headers hash. The new scheme allocates
Strings on insertion, but prevents most allocations on lookup by using
the equiv methods for querying by &str.

Fixes #12

@seanmonstar
Copy link
Member

I'm thinking keying with SendStr would be best. Keeping the header_name fn
returning static slices is ergonomic, and internally it can be wrapped into
a Slice. No need to allocate those strings for every single request. And no
user has to touch that HashMap, as it's Internal.

@reem reem changed the title Key Headers HashMap with String instead of &'static str Key Headers HashMap with SendStr instead of &'static str Sep 8, 2014
@reem
Copy link
Contributor Author

reem commented Sep 8, 2014

@seanmonstar You are right. I've updated the PR and commit.

This fixes an unsafe transmute that could cause use-after-frees
on collision resolution in the headers hash and avoids allocating
for cases where it is not necessary.

Fixes hyperium#12
seanmonstar added a commit that referenced this pull request Sep 8, 2014
Key Headers HashMap with SendStr instead of &'static str
@seanmonstar seanmonstar merged commit fcf2863 into hyperium:master Sep 8, 2014
@reem reem deleted the string-headers branch September 8, 2014 16:33
kstep added a commit to kstep/hyper that referenced this pull request Jan 10, 2015
ayax79 pushed a commit to ayax79/hyper that referenced this pull request Jan 14, 2015
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.

Transmute in headers does not look safe
2 participants