-
Notifications
You must be signed in to change notification settings - Fork 163
Improve /add interop #380
base: master
Are you sure you want to change the base?
Improve /add interop #380
Conversation
Signed-off-by: ljedrz <[email protected]>
…js-ipfs test case Signed-off-by: ljedrz <[email protected]>
There was a problem hiding this 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
// 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(()) | ||
}?; |
There was a problem hiding this comment.
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.
I browsed the
go-ipfs
andjs-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 onejs-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.