Skip to content

Conversation

apiraino
Copy link
Contributor

Diff rust-lang/mdBook@v0.4.48...v0.4.51

More specifically updates (transitively) the idna crate.

Hopefully this small update doesn't bring visual breaking changes (https://togithub.com/rust-lang/mdBook/pull/2608 is nice)

@rustbot
Copy link
Collaborator

rustbot commented May 27, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 27, 2025
@ehuss
Copy link
Contributor

ehuss commented May 27, 2025

Thanks! Can you say more about why you need to update the mdbook dependency in blacksmith? That won't update the version used for publishing. That is handled in deploy.yml.

Also, can you say why all the dependencies were updated instead of just mdbook?

@apiraino
Copy link
Contributor Author

hey thanks for the comments, you got me thinking again about it.

why you need to update the mdbook dependency in blacksmith? That won't update the version used for publishing. That is handled in deploy.yml.

So, I received a CVE advisory about idna (a dependency of mdbook) and thought to update it in all places mentioned by the advisory (i.e. cargo bisect, which you merged). I now wonder if this case is really necessary (as you point out it's not used other than locally) or maybe update the one in the deploy.yml? In any case the problem at hand (IIUC) is that a malicious punycode domain could sneak in a page, get through a review and be published on the forge.

but happy to hear your opinion, I'm not 100% clear on this.

Also, can you say why all the dependencies were updated instead of just mdbook?

that was probably because I always mixup cargo update/upgrade - I've forcepushed that again and now I think only mdbook-relevant deps are updated.

I'm also fine with closing this if you prefer! :)

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I went ahead and also updated the deployed version.

@ehuss ehuss merged commit 7310026 into rust-lang:master May 28, 2025
1 check passed
@apiraino apiraino deleted the update-dep branch May 29, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants