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

fix: block_put_post handle multipart bytes #186

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fdkevin0
Copy link

Code in Kubo rpc client use read_to_string handle file bytes, cause error like: Unable to build body: stream did not contain valid UTF-8.

match fields.read_to_string(&mut body_string) {

This issue also reappear in official test case:
async fn block_put() {
// Test data from:
// https://ipld.io/specs/codecs/dag-json/fixtures/cross-codec/#array-mixed
let cbor_cid =
Cid::from_str("bafyreidufmzzejc3p7gmh6ivp4fjvca5jfazk57nu6vdkvki4c4vpja724").unwrap();
// Cbor encoded bytes
let file = hex::decode("8c1b0016db6db6db6db71a000100001901f40200202238ff3aa5f702b33b0016db6db6db6db74261316fc48c6175657320c39f76c49b746521").unwrap();
let blob = Bytes::from(file.clone());
let mut mock_ipfs = MockIpfsDepTest::new();
mock_ipfs.expect_clone().once().return_once(move || {
let mut m = MockIpfsDepTest::new();
m.expect_put()
.once()
.with(
predicate::eq(cbor_cid),
predicate::eq(blob),
predicate::eq(vec![]),
)
.return_once(move |_, _, _| Ok(()));
m
});
let server = Server::new(mock_ipfs);
let resp = server
.block_put_post(
ByteArray(file),
Some(Codecs::DagCbor),
Some(Multihash::Sha2256),
Some(false),
&Context,
)
.await
.unwrap();
expect![[r#"
Success(
BlockPutPost200Response {
key: "bafyreidufmzzejc3p7gmh6ivp4fjvca5jfazk57nu6vdkvki4c4vpja724",
size: 57.0,
},
)
"#]]
.assert_debug_eq(&resp);
}
.
Using read_to_end could handle raw bytes and work well with my environment (ceramic-one as Kubo server).

@nathanielc
Copy link
Collaborator

@fdkevin0 Thanks for the PR! This is indeed a bug that we need to fix. However the code you have changed is part of the generated code for the server client logic. In order to fix this bug we will need to change the api definition here https://github.com/ceramicnetwork/rust-ceramic/blob/main/kubo-rpc/kubo-rpc.yaml#L228

You can use the make gen-kubo-rpc-server to regenerate the client with the code that can handle bytes vs utf-8 strings.

@fdkevin0
Copy link
Author

Sorry, I didn't realize that the code was generated using openAPI. I think it would be better to add a note saying Code generated - DO NOT EDIT at the top of the code file.
I edited the API definition and generated the code again. The current code runs correctly in my project. I'm not very familiar with openAPI, so could you please check if there are any issues?

@fdkevin0 fdkevin0 changed the title fix(kubo-rpc): block_put_post multipart with bytes fix: block_put_post handle multipart bytes Nov 16, 2023
@nathanielc
Copy link
Collaborator

I think it would be better to add a note saying Code generated - DO NOT EDIT at the top of the code file.

I agree, I wish the openapi generator would add that to every file it generates.

Something looks odd here. It seems while the client has changed to treating the file as raw bytes the server code has changed to treating the file as utf-8 bytes. Seems like there might be a bug in the openapi-generator itself.

I'll take a look and see if I can figure out what is up. Thanks again for finding this bug.

The current code runs correctly in my project.

Curious what your project is? Mind sharing any details? We currently are actively developing these APIs and they may change. Any input you have on them and your use case would be valuable to the team.

@fdkevin0
Copy link
Author

It seems while the client has changed to treating the file as raw bytes the server code has changed to treating the file as utf-8 bytes.

Yeah, the new generated code works properly when it is used with the new server code running daemon. However, I later discovered that it doesn't work properly when used with the ceramic-one latest container service. This may be due to a bug in the openAPI code generation tool. I also tried changing the format to file, but still couldn't generate the correct server-side code.

Curious what your project is? Mind sharing any details?

Sure, I'm currently working on the Dataverse file system, which is written in TypeScript and heavily uses the js-ceramic API. Our team is currently refactoring it using Rust to improve performance and meet our custom requirements. Specifically, I'm working on implementing the basic definition and parsing of ceramic commits (events). Once this work is completed, we would love to explore the possibility of merging it into your project.

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.

2 participants