marshallpierce / rust-base64

base64, in rust
Apache License 2.0
617 stars 115 forks source link

Replacement for base64::decode()? #233

Closed AndreKR closed 1 year ago

AndreKR commented 1 year ago

So base64::decode() is deprecated, but it's unclear to me what the replacement is.

The only related resource I found was issue https://github.com/marshallpierce/rust-base64/issues/213 where someone asked for base64::decode() to be brought back. As far as I understand that was declined because apparently there are multiple encodings called "base64" and we want to API to be explicit.

In the documentation I found examples that use base64::engine::general_purpose::STANDARD::decode() but when I tried that, decode() was not available:

image

Maybe the README could use a paragraph on what to replace base64::decode() with and/or how to find it in CLion's autocompletion

FrankenApps commented 1 year ago

There are multiple options. According to the Changelog I would personally go with this:

let data = base64::prelude::BASE64_STANDARD
            .decode(data_to_decode)
            .expect("Failed to decode base64 data.");

it will require the following trait in scope so that it works:

use base64::Engine;

With the trait in scope you should also be able to use base64::engine::general_purpose::STANDARD::decode() but it is a bit less concise...

AndreKR commented 1 year ago

it will require the following trait in scope

Do you know how I can get that in CLion without managing use statements manually?

FrankenApps commented 1 year ago

No I do not. I suspect, that this Open Issue is related.

However rust-analyzer in vsocde does correctly suggest the missing trait. The same applies for cargo check as can be seen [here](https://www.rustexplorer.com/b#%2F*%0A%5Bdependencies%5D%0Abase64%20%3D%20%220.21%22%0A*%2F%0A%0Afn%20main()%20%7B%0A%20%20%20%20let%20data_to_decode%20%3D%20%22abcdefgh%22%3B%0A%20%20%20%20let%20data%20%3D%20base64%3A%3Aprelude%3A%3ABASE64_STANDARD%0A%20%20%20%20%20%20%20%20%20%20%20%20.decode(data_to_decode)%0A%20%20%20%20%20%20%20%20%20%20%20%20.expect(%22Failed%20to%20decode%20base64%20data.%22)%3B%0A%7D).

marshallpierce commented 1 year ago

Using IntelliJ + Rust plugin (equivalent to CLion), in a new file typing base64::prelude::BASE64_STANDARD and then .dec autocompletes to .decode() and imports the Engine trait automatically.

evbo commented 1 year ago

@FrankenApps

use base64::{engine::general_purpose, Engine as _};

31  |     let bytes = base64::prelude::BASE64_STANDARD
    |  _______________-
32  | |             .decode("apTilujfQQA=");
    | |             -^^^^^^ method not found in `GeneralPurpose`

...also, why not just keep a helper function like base64::decode? If user needs to can always choose non-standard, but so many use cases just want to reproduce the result of a unix shell's base64 right?

marshallpierce commented 1 year ago

It sounds like whatever editor setup you have isn't properly suggesting trait methods, which is surely affecting many other types in Rust as well... Anyway, I can't reproduce the problem, and it seems to work fine for other users, so I'm closing this.

bjorn commented 1 year ago

Is there any chance of these methods coming back, possibly under a feature? Then people could consciously choose to go with the standard engine without cluttering crates everywhere with code like base64::Engine::decode(&base64::engine::general_purpose::STANDARD, some_bytes). :-(

If anything I think the docs could mention why the authors think the above is a good idea. It's a complete mystery to me why I should consciously choose between multiple possible base64 encodings. I haven't seen something like that in any other language where I've worked with base64 data.

marshallpierce commented 1 year ago

No; it is a disservice to users to hide the true complexity of seemingly simple things. The fact that you personally have not encountered different base64 alphabets or need for different underlying implementations is nice for you, but does not detract from the validity of other use cases. It is a better goal to give users the tools to precisely build correct code than it is to give users shortcuts that paper over complexity, increasing the possibility of building incorrect systems. So, the shortcuts will not return, and they never should have been implemented in the first place. You are welcome (and encouraged) to build your own, however, that are tailored specifically to your use case.

As to the unusual trait method syntax, I couldn't say why you're doing it that way (the crate certainly doesn't guide you to that), but you can always apply use to shorten or rename things as you prefer.

bjorn commented 1 year ago

As to the unusual trait method syntax, I couldn't say why you're doing it that way (the crate certainly doesn't guide you to that), but you can always apply use to shorten or rename things as you prefer.

I think the code is more readable when it is clear that some base64 encoding or decoding is happening. For readability, base64::encode was just perfect, whereas when I use this crate as documented, it becomes something like general_purpose::STANDARD_NO_PAD.encode(orig), which doesn't give any hint as to what encoding it really is, apart from it being apparently a "general purpose standard encoding without padding". :-S

No; it is a disservice to users to hide the true complexity of seemingly simple things. The fact that you personally have not encountered different base64 alphabets or need for different underlying implementations is nice for you, but does not detract from the validity of other use cases. It is a better goal to give users the tools to precisely build correct code than it is to give users shortcuts that paper over complexity, increasing the possibility of building incorrect systems.

I was indeed blissfully unaware about all the base64 variations I might need to care about. But I think the new approach is only making it worse. Now that I am aware of all the different options, what is going to help me choose the right option? When should I care whether encoding as base64 is adding padding or not? When should I change the alphabet used? I think all these options are more likely to drive me to make a wrong decision.

In general, an API is great when it makes easy things easy and hard things possible. It's great that your library supports all the various use-cases for base64 encoding, but it's sad that it no longer provides the quick and easy version that over 90% of your users are probably looking for. If you look at crates.io, you see that over half of your users are still using a version before 0.21, even 10 months after its release. There's projects unwilling to update due to the verbose clumsiness of the new API. There's people publishing new crates like https://crates.io/crates/easy_base64, https://crates.io/crates/simple-base64 and https://crates.io/crates/base64_light with the only benefit of providing a simpler API.

But this situation clearly sucks. I came here because I wanted to make sure my application only had a single base64-implementation among its dependencies. When the Rust community is going to switch to various simple crates or stick with older versions, then soon the dependencies of my app will be pulling in any number of base64 implementations. So the only solution I see is either for this library to add back the easy API in some way, or to just accept the complex one. :-/

marshallpierce commented 1 year ago

Now that I am aware of all the different options, what is going to help me choose the right option?

This is the problem. No crate can help you solve it. You must understand your requirements by reading the relevant specs, requirements, etc for your project, then choose the base64 variant you need. Pretending that this crate can make the choice for the user is the wrong choice. By the time the user is knowledgeable enough to use a particular shortcut, or not, they are also knowledgeable enough to not need it.

Losing users that are uninterested in precision and correctness is not a concern to me. I cannot help them if they do not wish to be helped. I hope to serve the users who do value those aspects, and aspire to build robust systems. I wish those other projects well, and I hope they enjoy the honeymoon period until they learn the same lessons.

bjorn commented 1 year ago

This is the problem. No crate can help you solve it. You must understand your requirements by reading the relevant specs, requirements, etc for your project, then choose the base64 variant you need.

Actually, I doubt it would help me even if I did read specs or requirements. In fact, most of the time, I will not have any specs or requirements but just need a simple way put binary data in a JSON or XML file.

Pretending that this crate can make the choice for the user is the wrong choice.

To be honest, the way I picked the API replacement, is by looking at the implementation of the deprecated method, and just inline it. It made me cringe, but at least it got rid of the deprecation warning. If the deprecated method would not have been available, I'd probably have picked it based on the namings "general_purpose" and "STANDARD". Because those names imply that all the others are (likely uncommon or specific) variations and the standard one is probably the one I need.

So regardless of there being a convenient method or not, this crate is going to make that choice for your users in its current form just as much as it was doing before. Nobody is going to see the deprecation warning and realize they've been using the wrong base64 variant all this time. All this deprecation did is to make life a little bit harder for everybody. :-(

marshallpierce commented 1 year ago

In fact, most of the time, I will not have any specs or requirements but just need a simple way put binary data in a JSON or XML file.

This is simply incorrect. You do have requirements. You may not understand them yet, but that doesn't mean they are not there.

This is an unproductively circular conversation.