Comments (5)
Awesome, thanks a lot for fixing that!
from jjwt.
Hi @michal-trnka! Thanks for the issue (and especially the test case!). This is very helpful.
For what it's worth, the concept of nested builders and using and()
is documented throughout the project:
- using a
Jwts.builder()
with the nestedheader()
builder: https://github.com/jwtk/jjwt?tab=readme-ov-file#jwtbuilder-header b64
non-detatched payload example: https://github.com/jwtk/jjwt?tab=readme-ov-file#non-detached-payload-example- Custom compression algorithm (
zip()
nested builder): https://github.com/jwtk/jjwt?tab=readme-ov-file#custom-compression-algorithm
Also individual JwtBuilder
claims methods work like this as well, they're just delegates to the nested claims()
builder. For example:
@Override
public JwtBuilder issuer(String iss) {
return claims().issuer(iss).and();
}
But I definitely understand how it may not be clear that one must call the and()
method, resulting in this unexpected behavior.
So we'll use this issue to track the work to address this. There are two solutions:
-
Change nested builder implementations to apply their changes immediately when
add
(and similar) methods are called.The challenge with this is that this is not typical builder behavior: most people expect builders to 'build' (do final completion work) when a
build()
method is called; (but for nested builders, theand()
method is essentially the build method, it just also returns to the parent builder).So changing builder pattern semantics (instant change per method vs change-all-when-build-is-called) might cause confusion. It's probably worth thinking through the repercussions of changing this.
-
Keep the existing "only apply changes when
and()
is called" and then just be super clear about this in documentation and JavaDoc so that it's consistent and people understand the repercussions.
I think my preference is the first solution if we can make this work easily and there are no negative side-effects. I'll have to do some testing.
Thanks again for the issue!
from jjwt.
Many thanks for the thorough explanation!
I have just again tested claims to be sure. It accepts the following code snipped. I guess this is what made me confused most.
builder.claims().add("my-claim", "my-value");
I'd prefer option # 2. However, adding it to JavaDoc would make the behavior more transparent and would work for me too.
from jjwt.
@michal-trnka after looking at the code, I went with the first option in PR #917 and it worked out well - no surprises or 'penalties' to work around.
There was partly a strong reason to apply changes immediately, not the least of which is that it didn't 'hurt' anything, but also it avoids problems with method chains like:
.header().add("foo", "bar");
// sometime later:
.header().empty().critical()...
If we didn't apply changes immediately when .empty()
is called, we'd have to create internal 'stack' of sorts to track changes across method invocations before the .and()
method is finally called. It was much easier to avoid that and be consistent across all mutation/change methods.
from jjwt.
This is now released in 0.12.5
. @michal-trnka Thank you for reporting the issue!
from jjwt.
Related Issues (20)
- Disable Jackson ObjectMapper FAIL_ON_UNKNOWN_PROPERTIES Deserialization Feature HOT 1
- Unable to extract claims when cty specified in JWS header (>0.12.0) HOT 19
- Question: How far is version 1.0 (due to breaking changes in 0.12)?
- NIST Elliptic Curve JWK field element octet string padding HOT 1
- Allow parsing signed JWTs without the key HOT 1
- Impossible to build a JWK with `alg: HS512` and a `k` that is larger than 64 bytes HOT 2
- Make JacksonDeserializer constructor public
- Unable to access jcaName from Jwts.SIG HOT 2
- Consider a convenience method to obtain a `Jwk` from a `JwkSet` by Key ID HOT 4
- Deprecation of support for unsecured JWT token parsing with algo != 'none' HOT 3
- NoSuchMethodError when using libraries built against older jjwt version HOT 2
- Unable to verify Elliptic Curve signature using configured ECPublicKey. Invalid encoding for signature HOT 2
- CVE reported against 0.11.5 HOT 5
- CVE-2024-31033 (v0.12.5) HOT 2
- java.lang.IllegalArgumentException: Invalid Map 'iv' (Initialization Vector) value: 1230868678. Values must be either String or [B instances. Value type found: java.math.BigInteger. HOT 4
- JWE arbitrary content compression/decompression error
- X5C header does not accept a chain of certificates longer than 1 HOT 6
- Algorithm Request/Result Builders
- AUD as only array breaks legacy/existing implementations HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from jjwt.