-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
chore(deps): bump actix-multipart from 0.6.2 to 0.7.2 #315
Conversation
@orhun can you help me with this? I was able to fix it by adding an let content = ContentDisposition::from(field.content_disposition().unwrap().clone()); The tests pass, but using unwrap is never a good idea. How can this be properly fixed? I am not even sure why I had to make changes in the first place. There's nothing in the actix-multipart release notes that would expain why this errors out without the unwrap. |
I've read up on this a bit. It seems that the Per RFC 7578 §4.2, the parts of a multipart/form-data payload MUST contain a Content-Disposition header field where the disposition type is As such, it is safe to I still wanted to get your input on this. |
Bumps [actix-multipart](https://github.com/actix/actix-web) from 0.6.2 to 0.7.2. - [Release notes](https://github.com/actix/actix-web/releases) - [Changelog](https://github.com/actix/actix-web/blob/v0.7.2/CHANGES.md) - [Commits](actix/actix-web@v0.6.2...v0.7.2) --- updated-dependencies: - dependency-name: actix-multipart dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
aadcf5f
to
ef1092c
Compare
Yup, it seems correct. We can simply: diff --git a/src/server.rs b/src/server.rs
index fc37be5..7e1173d 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -225,7 +225,14 @@ async fn upload(
while let Some(item) = payload.next().await {
let header_filename = header::parse_header_filename(request.headers())?;
let mut field = item?;
- let content = ContentDisposition::from(field.content_disposition().clone());
+ let content = ContentDisposition::from(
+ field
+ .content_disposition()
+ .ok_or_else(|| {
+ error::ErrorInternalServerError("payload must contain content disposition")
+ })?
+ .clone(),
+ );
if let Ok(paste_type) = PasteType::try_from(&content) {
let mut bytes = Vec::<u8>::new();
while let Some(chunk) = field.next().await { |
I am not sure what is going on, but if I run
Same happens when I check out this PR and run the above cargo commands. P.S.: This did not happen when I tested the unwrap. |
I got the same error after upgrading to Rust 1.80.0. I think there is a dependency incompatibility somewhere in the chain. Might be worth cleaning the cargo cache and retrying too (I can't because it's a global cache 💀) |
I cleared the cache in my local project tree and it didn't help. |
4c40c8b should fix it. |
Hmm, we do not use the time crate directly. So whatever other crate we are using that requires time as its dependecy will have to update it. So we have to wait for that. |
It looks like actix-web and bunch of other crates are using it. I don't think we should wait, you can simply cherry-pick that commit to fix the CI. |
Ha, ok. So you hacked the Cargo.lock manually. Hmm, wouldn't that be overwritten when there are changes to the Cargo.toml? |
I don't think so. I simply ran |
Interesting. Thanks for the info. Ahh, it probably would only be a problem, if a crate were using an inequality requirement. |
Hmm, I can't push changes to this PR. I guess I will have to create a new PR with your suggested changes. I will do this tonight or tomorrow. |
@dependabot close |
Bumps actix-multipart from 0.6.2 to 0.7.2.
Release notes
Sourced from actix-multipart's releases.
Changelog
Sourced from actix-multipart's changelog.
... (truncated)
Commits
b4ed564
update changes80fbc2e
Fix stream draining for http/2 connections #290f580650
fix missing content-encoding header for h2 connections #4216048817
Correct flate feature names in documentatione408b68
Update cookie dependency (#422)b878613
fix warning85b275b
fix warningsd6abd2f
allow to handle empty path for application with prefixb79a9aa
fix changelogb9586b3
Merge pull request #412 from gdamjan/masterDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)