Load PSBTs using istreambuf_iterator rather than istream_iterator #687
Conversation
istream_iterator eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
hebasto
changed the title
There was a problem hiding this comment.
This is a regression in 24.0.
It would be nice to add a test to prevent such a regression in the future.
Perhaps you could be interested in reviewing of this PR as an author/a committer of bitcoin/bitcoin@a65931e in bitcoin/bitcoin#25001.
| @@ -212,7 +212,7 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) | |||
| return; | |||
| } | |||
| std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary}; | |||
| data.assign(std::istream_iterator<unsigned char>{in}, {}); | |||
| data.assign(std::istreambuf_iterator<char>{in}, {}); | |||
There was a problem hiding this comment.
This change reverts bitcoin/bitcoin@a65931e partially, in particular:
std::istream_iterator-->std::istreambuf_iteratorunsigned char-->char
But the local std::vector data is still parameterized by unsigned char:
Line 195 in bb5ea1d
Is it on purpose?
There was a problem hiding this comment.
Yes, that was intentional. Using unsigned char does not appear to work (fails to compile). But I don't think that data type makes a difference here.
There was a problem hiding this comment.
Yeah, std::ifstream is std::basic_ifstream<char>, and passing that to std::istreambuf_iterator can only be done if it is templated <char> as well. C++ (17?) derives this automatically, so you can also write:
data.assign(std::istreambuf_iterator{in}, {});
There was a problem hiding this comment.
Docs: https://en.cppreference.com/w/cpp/iterator/istream_iterator#Notes
review ACK bb5ea1d 🍇
Show signature
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK bb5ea1d9a954b7b9f443ee8fbbb04549cd0b08a7 🍇
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUg34wv+NFe6qKxea+Z5Jx0PTNpbWXv+nTGMQYsOek6a+pJIe00MuJGO4XIuDJS0
Mffqqi8fhC83It02w8Z6q9zLSXB094GabCYgFanN5g0AxDdwgvjrEM5nQ9cZj3Vo
hJHDzVpdgag5i311J+05yeHig9EOnRjxBRgf1p3pv+/Tg3yS/M7UIRKdpmqAxfOA
HdPMjRQr+Tjm7+/VHwJJpEnE0fPLUZX5NjblKUmG4wDIwlcqz0qYUK8JkAtznYz9
JJvXxOayoGKNk3IF/UfZs0/cmGlPh+5AJ3asnSIizAITiwIOFaNAIUfprgc4d86V
fo66d2avigfwHVpKsLsbQep52I0deYo1dItSGvhcYxhtunB/D8eBsMi8b5kk489j
kV6i1kP/VtUVu2v2Pkgl632aupAcWM62gY3YCl50IfqNfpMgf0uG58NEScZFHmbp
920c3zvXD9n7SDx3GVQPljssLPBG7WMUfBYmwYP0NtWx28qv250dThWYnOUUoour
1GDZDw4r
=QuKV
-----END PGP SIGNATURE-----
istream_iterator eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters. Github-Pull: bitcoin-core/gui#687 Rebased-From: bb5ea1d
…er than istream_iterator bb5ea1d qt: Load PSBTs using istreambuf_iterator rather than istream_iterator (Andrew Chow) Pull request description: `istream_iterator` eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters. `istreambuf_iterator` is the correct thing to use here. This is a regression in 24.0. bitcoin#25001 accidentally changed the original `istreambuf_iterator` to `istream_iterator`. ACKs for top commit: furszy: Tested ACK bb5ea1d MarcoFalke: review ACK bb5ea1d 🍇 Tree-SHA512: 35d90eee3efdcb6a360af69ac1727f9f2837ea621297196de3136299f5de6d9975df4e425e1fc5b8813c1ddb2a4d60c3969e1d5d968953a4628ca45e37d3bf05
|
Removing label as this was backported: bitcoin/bitcoin@0662105. |
istream_iteratoreats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters.istreambuf_iteratoris the correct thing to use here.This is a regression in 24.0. bitcoin/bitcoin#25001 accidentally changed the original
istreambuf_iteratortoistream_iterator.