Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

Improve /add interop #380

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Improve /add interop #380

wants to merge 2 commits into from

Conversation

ljedrz
Copy link
Member

@ljedrz ljedrz commented Sep 18, 2020

I browsed the go-ipfs and js-ipfs codebases in search for some nice /add test cases so that we could verify that our logic, e.g. wrt. the headers and field names, is compatible; I haven't found much 😅, but based on the one js-ipfs test case I found I relaxed the /add restrictions a little bit.

Not sure if this is the right approach and if I'm seeing the whole picture, so I'm marking this as a draft.

Copy link
Collaborator

@koivunej koivunej left a comment

Choose a reason for hiding this comment

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

comments


tokio::spawn(fut);
ipfs
assert_eq!(response.status(), 200);
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this test case is just to accept the text/plain as I guess js-ipfs accepts it?

accepting text/plain sounds really strange be honest ... wouldn't that require some charset wrangling as well? I guess there's no issues if you assume ascii or utf8 but ... Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

this was the way js-ipfs did it, I just followed suit; since it's random alphanumeric bytes there's no encoding issues

Comment on lines -148 to -153
// files are of the form "file-{1,2,3,..}"
let _ = if field_name != "file" && !field_name.starts_with("file-") {
Err(AddError::UnsupportedField(field_name.to_string()))
} else {
Ok(())
}?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if this is a good idea to be removed. I mean if you think of a html form which is pretty much the basis for http multipart and fields with different names kinda should be different things so I'd like to retain some validation OR targeting here. Or are the go-ipfs and js-ipfs using random field names here? I'd rather see them "enumerated" here.

my comment is a bit off here, I think js-ipfs-http-client does the file-N.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants