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

get_mut doesn't seem to work #821

Closed
Manishearth opened this issue Jun 7, 2016 · 3 comments
Closed

get_mut doesn't seem to work #821

Manishearth opened this issue Jun 7, 2016 · 3 comments
Assignees
Labels
A-headers Area: headers. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@Manishearth
Copy link
Contributor

Manishearth commented Jun 7, 2016

I'm not sure what happened here:

extern crate hyper;
use hyper::header::Headers;
use hyper::header::{ContentLength, ContentType};
use hyper::http::RawStatus;
use hyper::method::Method;
use hyper::mime::{Attr as MimeAttr, Value};

fn main() {
    let mut x = Headers::new();
    x.set_raw("Content-Type", vec!["text/plain;charset=shift-jis".bytes().collect()]);

    println!("old: {:?}", x); // charset is shift-jis
    if let Some(old_ct) = x.get_mut::<ContentType>() {
        println!("old old_ct: {:?}", old_ct); // still shift-jis
        let mut new_ct = old_ct.clone();
        for param in (new_ct.0).2.iter_mut() {
            if param.0 == MimeAttr::Charset {
                println!("old param {:?}", param); // shift-jis
                *param = (MimeAttr::Charset, Value::Ext("hi".into()));
                println!("new param {:?}", param); // hi
            }
        }
        *old_ct = new_ct;
        println!("new old_ct: {:?}", old_ct); // hi
    }

    // at this stage I'd expect the header to have changed. But it hasn't.
    println!("new get: {:?}", x.get::<ContentType>()); // hi
    println!("new get raw: {:?}", x.get_raw("content-type")); // shift-jis again??
    println!("new: {:?}", x); // shift-jis again??
}
@Manishearth
Copy link
Contributor Author

I guess the mixture of raw and non-raw methods is breaking it, but I'm not sure. Does set_raw create two copies of the thing -- one typed, one untyped?

@Manishearth
Copy link
Contributor Author

I guess the issue is that typed_mut doesn't remove the header from the raw headers when inserting a typed one.

@seanmonstar
Copy link
Member

Ah, this is a bug. The invariants I've been enforcing are: if you get an immutable view (raw or typed), and it doesn't exist, it will generate it and then give it to you (either serializing a typed, or parsing a raw). However, if you get a mutable reference, because you can cause them to be out of sync, it should obliterate the other stored value. So, get_mut should kill the raw part. I believe it does this correctly when you do headers.set(Foo), so it's a hole in get_mut.

@seanmonstar seanmonstar added C-bug Category: bug. Something is wrong. This is bad! A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor. labels Jun 13, 2016
andresilva added a commit to andresilva/hyper that referenced this issue Jun 17, 2016
…eader

If you get a mutable reference to a typed header it is possible to make
the two representations be out of sync. To avoid this, after parsing the
raw part it should be removed.

Fixes hyperium#821.
andresilva added a commit to andresilva/hyper that referenced this issue Jun 17, 2016
… header

If you get a mutable reference to a typed header it is possible to make
the two representations be out of sync. To avoid this, after parsing the
raw part it should be removed.

Fixes hyperium#821.
seanmonstar pushed a commit that referenced this issue Jun 20, 2016
… header

If you get a mutable reference to a typed header it is possible to make
the two representations be out of sync. To avoid this, after parsing the
raw part it should be removed.

Fixes #821.
@seanmonstar seanmonstar self-assigned this Jun 20, 2016
seanmonstar pushed a commit that referenced this issue Jun 21, 2016
… header

If you get a mutable reference to a typed header it is possible to make
the two representations be out of sync. To avoid this, after parsing the
raw part it should be removed.

Fixes #821.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-headers Area: headers. C-bug Category: bug. Something is wrong. This is bad! E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Development

No branches or pull requests

2 participants