Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation errors (VS 2022 32-bit /W3 /SDL) #278

Open
Kemp-J opened this issue Feb 6, 2023 · 6 comments
Open

Compilation errors (VS 2022 32-bit /W3 /SDL) #278

Kemp-J opened this issue Feb 6, 2023 · 6 comments

Comments

@Kemp-J
Copy link

Kemp-J commented Feb 6, 2023

When building a project that uses the jwt-cpp header in Visual Studio 2022 with warning level 3 and SDL checks enabled there are several errors raised related to suspicious implicit casts. These can be easily solved with a few tweaks. Specifically:

  • The value returned by jwt::alphabet::index is currently a uint32_t, which is smaller than the ptrdiff_t that the call to std::distance returns. The return value can be switched to size_t to solve this.
  • The sextet_* and triple variables in jwt::base::details::decode call the above and so need to be size_t as well.

In modern C++ both of the above points can be solved with auto in preference to specifying the types.

  • The argument passed to system_clock::from_time_t in jwt::basic_claim::as_date requires a static_cast to a time_t.

I was going to raise an issue about a common base class for exceptions, but it looks like someone got there first :)

@prince-chrismc
Copy link
Collaborator

Hmm, never heard of /sdl very cool see msvc getting way better.

I would love to see a PR with these fixes 🙏

@Kemp-J
Copy link
Author

Kemp-J commented Feb 6, 2023

I'll see what I can do. I use git elsewhere but I'm quite new to using it with github 😬

@prince-chrismc
Copy link
Collaborator

It's the same tbh, just create a fork (there a button top left) then git clone your copy, and the rest is the same.
After you push you changes to a branch, this repo will have a "open pull request banner" if not the pull request tab has a "new" button :)

It sounds like a lot but the CLI will even output hints along the way with links!

@Kemp-J
Copy link
Author

Kemp-J commented Feb 10, 2023

A followup issue - using nlohmann json with jwt-cpp has additional errors for 32-bit builds. The source of this is that is_substr_start_end_index_signature and is_substr_start_index_signature check for the existence of substr member functions on string_type, but they do so on the assumption that they will accept integer_type for the parameters. The integer_type for nlohmann json is int64_t, the substr function defined for std::string has size_t parameters, and thus the error as the compiler sees you effectively trying to pass int64_t values to a size_t parameter (unsigned int on 32-bit builds). Given that, as far as I can tell, you don't actually need substr to accept integer_type specifically, is it worth swapping the check to look for substr taking size_t?

@prince-chrismc
Copy link
Collaborator

Windows 32-bit?! I've not touched that in 8 years :) I have no idea.

@Kemp-J
Copy link
Author

Kemp-J commented Feb 11, 2023

Yeah, I have a few things I have to build as 32-bit for... historical reasons.

I guess there's mainly a question of intent here. If there is a reason substr needs to take integer_type, because it's called with values directly from a token, then it'll need some creativity. I don't think your calls to substr are used like that though (pretty much just hardcoded values and ones calculated from other sources) so moving the check away from integer_type wouldn't cause any problems. If you have insight on anything I missed then I'd be happy to hear it, otherwise I'll roll that into my changes. Might as well have 32-bit working if it only requires a small tweak here and there 🙂

@prince-chrismc prince-chrismc changed the title Compilation errors (VS 2022, W3, SDL) Compilation errors (VS 2022 32-bit /W3 /SDL) Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants