quick-xml icon indicating copy to clipboard operation
quick-xml copied to clipboard

enhancement: return bytes written from `write_event`

Open phdavis1027 opened this issue 2 years ago • 5 comments

Writer now reports the number of bytes written by the underlying Write. My use case is that I need to send a prelude over TCP reporting how long serialized XML messages will be before I send the actual message.

phdavis1027 avatar Feb 28 '24 17:02 phdavis1027

Turns out I missed write_indent, which is what caused the fuzz to fail. Fixed that and also added a changelog entry, although I'm not sure I got it in the correct format. If you think this is good I can squash the intermediate commits.

phdavis1027 avatar Feb 28 '24 18:02 phdavis1027

And reformat, and fix the new fuzzing failure

dralley avatar Feb 28 '24 18:02 dralley

Codecov Report

Attention: Patch coverage is 67.27273% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 65.81%. Comparing base (adf873e) to head (1fa8dff). Report is 1 commits behind head on master.

Files Patch % Lines
src/writer.rs 67.27% 18 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #721      +/-   ##
==========================================
+ Coverage   65.46%   65.81%   +0.34%     
==========================================
  Files          38       38              
  Lines       18165    18231      +66     
==========================================
+ Hits        11892    11999     +107     
+ Misses       6273     6232      -41     
Flag Coverage Δ
unittests 65.81% <67.27%> (+0.34%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Feb 28 '24 18:02 codecov-commenter

Actually the fuzzer has caught a harder case -- write_text_content already has a non-empty type for its result. First thought that jumps to mind is to just return a Result<(usize, &'a mut Writer<W>)> but obviously that breaks the kind of chaining used in the tests. EDIT: I just noticed I can run the fuzzer locally. Sorry, not too experienced with open source.

phdavis1027 avatar Feb 28 '24 18:02 phdavis1027

About the original purpose - are you serializing the XML out to a buffer first? If so, wouldn't it be easier to just get the length of the data in the buffer?

dralley avatar Mar 01 '24 00:03 dralley

About the original purpose - are you serializing the XML out to a buffer first? If so, wouldn't it be easier to just get the length of the data in the buffer?

It's very possible I'm missing an obvious alternative solution. Ideally, I'd pack multiple things into one buffer in reverse order (i.e., the last thing serialized is the first element in the buffer.) For example the Header lives somewhere in buf[4..MAX_HEADER_LEN], but I don't know how to determine which slice to send without knowing how many bytes were actually written there by the serializer.

Also, sorry it slipped by this weekend. I'm running the fuzzer on a commit with just the original changes right now but it really beats up my crummy HP. Unless it would blow up y'all's notifications too much I might have to finish this out tomorrow with some test runs on Github.

phdavis1027 avatar Mar 05 '24 02:03 phdavis1027

I'm going to second-guess myself here: I just discovered you can use std::io::Cursor for basically the same purpose. Doesn't that make this PR redundant?

phdavis1027 avatar Mar 11 '24 18:03 phdavis1027

I didn't think of io::Cursor specifically but that's kind of what I was wondering with my previous comment. But you're right, io::Cursor does do something very similar. I was thinking that perhaps you might not even need that - if you're writing to Vec<u8> for instance you could just check the length of the Vec<u8>. io::Cursor is a more general solution, though.

I probably could have been more specific about that, sorry.

Anyway, my suggestion would be to try out io::Cursor and see if you can find any use case that it doesn't handle. And if it turns out that you can accomplish everything you need to without this PR then we can ice it for now.

dralley avatar Mar 11 '24 19:03 dralley

I didn't think of io::Cursor specifically but that's kind of what I was wondering with my previous comment. But you're right, io::Cursor does do something very similar. I was thinking that perhaps you might not even need that - if you're writing to Vec<u8> for instance you could just check the length of the Vec<u8>. io::Cursor is a more general solution, though.

I probably could have been more specific about that, sorry.

Anyway, my suggestion would be to try out io::Cursor and see if you can find any use case that it doesn't handle. And if it turns out that you can accomplish everything you need to without this PR then we can ice it for now.

Thanks for the suggestion. In my particular case I'm writing to a &mut [u8], but it does turn out that Cursor can do everything I need it to. The overhead of creating the Cursor is so minimal that I doubt anyone would need other solutions. Sorry for the frivolous PR!

phdavis1027 avatar Mar 13 '24 17:03 phdavis1027

std::io::Cursor::position() can report how many bytes any Writer wrote. See the following:

let mut cursor = Cursor::new(buf);
let mut writer = Writer::new(&mut cursor);
... // Write a bunch of stuff
// Once you're done writing...
Ok(cursor.position() as usize)

phdavis1027 avatar Mar 13 '24 17:03 phdavis1027

No worries at all! Thanks for the time & discussion.

dralley avatar Mar 13 '24 17:03 dralley