std::string_view support

[sjanel]

If the json library supports std::string_view, allow jwt-cpp take builder parameters as std::string_view and algorithm constructors as well.

Also small optimizations in some methods of jwt.h to make sure only one object is returned so that RVO can be used.

PR is ready for review. Only the coverage is failing, I will check why.

[sjanel]

I generally understand the motivation behind this contribution but I would generally suggest opening an issue to discuss ideas this large, tip for the future :)

There are a handful of changes which are a little unclear what the motivation so I left some comments during an initial pass.

Overall the code looks really good, my comments are for existing users who might want to update as well as some performance consideration :)

Thanks for the comments, I was quite busy these days, but I will try to come back on my PR, I did not forget it ;) I will try to split it as well. I think I can extract some stuff in base.h first.

[Thalhammer]

For some reason I could not reply to a bunch of your comments, so here we go.

It's at least equivalent considering the alphabets it receives. Do we expect the alphabets to change ? As this file is meant to be internal the risk is close to 0 to introduce a bug here (and it's unit tested anyway).

Likely not, but its exposed to the public and someone might use it with a different alphabet. Its a user visible header and the base64 stuff is not in a details namespace which makes it kinda public api (even if its not advertised as one). If we potentially break peoples code, we should at least break it at the compile stage (I.e. change the types inside the alphabets and remove the - no superficial - first 62 chars) instead of silently changing behavior in edge cases.

If it's no longer maintained, why is it a problem anyway as it should be removed from this repo ?

Mostly because it being there didn't hurt so far and removing it would likely break a lot of users codebase because its the default library. Also having one in there that does not rely on a third party json library makes it way easier to use (no extra dependencies).

[sjanel]

Hello @Thalhammer @prince-chrismc

I answered your comments, not sure what is preventing the PR to be merged currently.

Also, it fixes following gcc 12 compiling error:

[build] include/jwt-cpp/base.h: In static member function ‘static const std::initializer_list<std::__cxx11::basic_string<char> >& jwt::alphabet::helper::base64url_percent_encoding::fill()’:
[build] include/jwt-cpp/base.h:86:100: error: ‘(((const std::__cxx11::basic_string<char>*)(& <temporary>)) != 0)’ is not a constant expression
[build]    86 |                                         static std::initializer_list<std::string> fill{"%3D", "%3d"};

and an attempt to fix it 'easily' is more complicated than expected.

Here with this PR and string_view it's much easier.

EDIT: sorry I did not catch your last global comment. I can revert the changes in the alphabet if needed. However I don't see how to avoid picojson code changes (see my below comment)

[sjanel]

Mostly because it being there didn't hurt so far and removing it would likely break a lot of users codebase because its the default library. Also having one in there that does not rely on a third party json library makes it way easier to use (no extra dependencies).

That's the reason why I fixed it instead of removing it. They are not complicated changes, and backward compatible with ancient compilers. If we cannot remove it, and cannot change it, how can we move on ?

[prince-chrismc]

I am sincerely sorry, I've been meaning to follow up but I am currently between jobs and trying to move countries. It might be several more weeks before I have a chance to work from my computer so I'd really appreciate your patients.