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

bump ring to 0.14.0 #110

Merged
merged 1 commit into from
Feb 11, 2019
Merged

bump ring to 0.14.0 #110

merged 1 commit into from
Feb 11, 2019

Conversation

ubnt-intrepid
Copy link
Contributor

No description provided.

@landesfeind
Copy link

Hi, would it be possible to go forward with this request? As far as I can tell, the Travis checks for nightly builds failed due to the inability to install nightly.
Bumping this version would be very helpful!

@ubnt-intrepid
Copy link
Contributor Author

@alexcrichton could you re-run the CI build?

@alexcrichton
Copy link
Collaborator

cc @SergioBenitez

@SergioBenitez
Copy link
Member

It looks like we're getting a true failure. Can you investigate, @ubnt-intrepid?

@ubnt-intrepid
Copy link
Contributor Author

ubnt-intrepid commented Jan 16, 2019

@SergioBenitez The failure is caused by Iter returning removal Cookie (it is reproduced on master branch and hence is not unique problem of this PR). the failure occurs only on nightly toolchain, and hence it might be a bug in the standard library.

Test code (modification of Iter's doctest):

#[test]
fn issue() {
    let mut jar = CookieJar::new();

    jar.add_original(Cookie::new("name", "value"));
    jar.add_original(Cookie::new("second", "two"));

    jar.add(Cookie::new("new", "third"));
    jar.add(Cookie::new("another", "fourth"));
    jar.add(Cookie::new("yac", "fifth"));

    jar.remove(Cookie::named("name"));
    jar.remove(Cookie::named("another"));

    for cookie in jar.iter() {
        eprintln!("({}, {})", cookie.name(), cookie.value());
    }
}

Expected output (the result on stable/beta):

(new, third)
(yac, fifth)
(second, two)

The result on nightly:

(name, value)
(second, two)
(yac, fifth)
(new, third)

(edited) this failure occurs from nightly-2019-01-16, i.e. the latest nightly release at the present time.
It is possible that this patch causes this regression.

@ubnt-intrepid
Copy link
Contributor Author

Due to the above patch, there is a possibility that the reference to DeltaCookie received in Iter will be from (unmodified) original_cookies. As a result, the validation in Iter::next is always true and Iter returns its reference even if it is a removed Cookie.

@SergioBenitez
Copy link
Member

SergioBenitez commented Jan 17, 2019

@ubnt-intrepid Thanks for investigating that! Can you rebase on the latest master to rerun tests?

Edit: Forgot I could do this! Just did.

@ubnt-intrepid
Copy link
Contributor Author

@SergioBenitez Thanks!

@lucab
Copy link

lucab commented Jan 26, 2019

Gentle ping to get this reviewed, merged and released.
(there is a bump for the native library ring-asm hiding in here, which makes updates in consumers dependent on having all libraries updated first)

@Darkspirit
Copy link

It looks like crates.io itself is waiting for this: briansmith/ring#774 (comment)

@sgrif
Copy link

sgrif commented Jan 26, 2019

@Darkspirit We're actually not on the latest version of cookie, we'll have to update some unmaintained dependencies to get there (and I'm not sure exactly how much work that'll be). But yeah, y'all may want to move on this sooner rather than later since the author is intending to break all crates using 0.13.5 soon. briansmith/ring#774 (comment)

@dwijnand
Copy link

I agree, cutting a new version of cookie with this would probably help the ecosystem recover from the ring versions purge.

@briansmith
Copy link
Contributor

briansmith commented Jan 29, 2019

But yeah, y'all may want to move on this sooner rather than later since the author is intending to break all crates using 0.13.5 soon. briansmith/ring#774 (comment)

Please, it's unfair to state this like I'm purposely trying to break people's projects. Now that I'm aware of the Cargo problem (rust-lang/cargo#6609), I may delay yanking 0.13.5.

Regardless, it is good to use ring 0.14 because then AES-GCM will start working on AAarch64 devices that don't have the PMUL instruction. Right now AES-GCM doesn't work on devices without that instruction, including in particular Raspberry Pi 3.

@SergioBenitez SergioBenitez merged commit c41fe46 into rwf2:master Feb 11, 2019
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.

9 participants