Remove invalid but extant base64 protocol test case #1168
Conversation
While `YmxvyG=` is _technically_ invalid base64 according to the spec (its Unicode code point length is not divisible by 4), it can always be unambiguously decoded, even over a network protocol that concatenates base64-encoded strings and that would hence require padding characters. Some off the shelf base 64 implementations, like the canonical `base64` Rust crate (https://docs.rs/base64/latest/base64/), decode this case successfully.
|
@david-perez if we're going to be more accepting of base64 than the tests allowed previously, we have the same sort of portability problem, but now undocumented. Can you add a positive request test forcing implementations that are strict about padding to correct the padding before decoding? |
|
@adamthom-amzn A positive request test might break other off the shelf implementations. Does Smithy want to take a stance on this issue and enforce being lenient in what a server accepts? |
|
It has to pick one or the other. If Rust is more lenient than TS, then the same smithy model cannot host a server with the same behavior on both backends. |
|
The same argument goes for #1116 and it was closed; it's another source of possible discrepancies among server implementations. Personally, if Smithy has to take a stance on this, I'd prefer that it adopt being lenient in what it accepts. |
|
I don't agree that #1116 is the same - it's controlled by the HTTP spec, so there's no need for Smithy to additionally constrain it, whereas base64 in the contexts we accept it in is not controlled by any particular spec I am aware of. I also don't really like the robustness principle - after all, it's also an excuse for some of our RPC protocols to accept "y", "n", 1, and 0 for booleans. That's now a requirement for every implementation of those protocols even though very few JSON parsers do that out of the box. It also means that every implementation of our HTTP binding protocols is backwards-incompatibly stricter than our existing implementation, because it will accept any date in any format in any location. |
|
I don't understand how #1116 is not the same in the strict sense that it is a source of possible discrepancies among server implementations. The HTTP spec is unrelated to the fact that Smithy accepts binding any integer as an HTTP response code without prescribing what to do at runtime when a said integer falls outside the allowed range. I'm also not fond of Postel's law being cited arbitrarily. In any case, if Smithy team decides it wants to be strict in accepting base64, this PR should be reverted and the documentation should be expanded. If we want to be lenient, I'll submit a positive test case on Monday. |
|
My commit message is incorrect: it cites |
|
After reading https://eprint.iacr.org/2022/361, I am now convinced that this test case should not be removed and that base64 decoding implementations should be strict about padding. |
While
YmxvyG=is technically invalid base64 according to the spec(its Unicode code point length is not divisible by 4), it can always be
unambiguously decoded, even over a network protocol that concatenates
base64-encoded strings and that would hence require padding characters.
Some off the shelf base 64 implementations, like the canonical
base64Rust crate (https://docs.rs/base64/latest/base64/), decode this case
successfully.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.