Hi there. Congrats on 1.0 release, I'm excited to migrate all my code 😂 (actually though!)
The codebase I encountered this situation in is still on an older version of typebox, but the typebox code that I tracked it back to seems to still be present in master, so I'm filing this anyway.
--
I encountered something unexpected while working on some code performing OAuth2 duties. Since the spec allows for auth via either headers or body parameters, I created a schema that is essentially "maybe a given type, or maybe a given type plus the two body parameters":
const TMaybeBodyAuth = <T extends TProperties>(properties: T) =>
T.Union([
// Basic Auth is preferred
// https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1
T.Object({
...BodyAuth,
...properties,
}),
T.Object(properties),
]);
I've also created a type for holding secrets in a weakmap, to avoid accidentally exposing them, and used a transform type to handle wrapping/unwrapping secrets as early/late as possible:
export const TSecret = <T extends TSchema>(type: T, label?: string) => {
return T.Transform(type)
.Decode(
(value: StaticEncode<T>): Secret<StaticDecode<T>> =>
new Secret(value, label)
)
.Encode(
(secret: Secret<StaticDecode<T>>): StaticEncode<T> => secret.unwrap()
);
};
I finally got to a point where I'm doing end-to-end testing against an OAuth server, and was surprised to be getting 4xx errors saying "invalid client secret". After some debugging, I found that the body did not include the unwrapped secret, for the following combination of reasons:
- The data I wanted it to encode satisfies both members of the union
- Typebox (presumably for performance reasons) checks for a valid schema from all union members with untransformed data, and only then re-attempts with the transformed versions:
|
// test value against union variants |
|
for (const subschema of schema.anyOf) { |
|
if (!Check(subschema, references, value)) continue |
|
const value1 = Visit(subschema, references, path, value) |
|
return Default(schema, path, value1) |
|
} |
|
// test transformed value against union variants |
|
for (const subschema of schema.anyOf) { |
|
const value1 = Visit(subschema, references, path, value) |
|
if (!Check(schema, references, value1)) continue |
|
return Default(schema, path, value1) |
|
} |
|
return Default(schema, path, value) |
- I had ordered the union members backwards (the "shorter" one first), but this turned out not to matter
- I had not specified
{ additionalProperties: false }
Setting additionalProperties to false on the object does produce the expected result:
(with additionalProperties: false)
value: {
client_id: 'foo',
client_secret: 'bar',
grant_type: 'client_credentials'
}
(without additionalProperties: false)
value: {
client_id: 'foo',
client_secret: Secret(client_secret),
grant_type: 'client_credentials'
}
... so this isn't exactly a bug, just a fairly sharp corner that I banged my knee on.
While the behavior is explicable and makes sense in light of my discoveries, it might be worth considering different logic here. If I'm explicitly calling "Encode" on some data, I am not expecting it to return unencoded data -- so it's surprising to me that the first loop exists at all. The Encode function also appears to operate on the data in-place, so my output was including the client_id and client_secret properties because they were present on the input, but their content was unexpected because the schema being applied was not the one I expected. (This may have changed in the new version, but I have a lot of work to do before I can verify experimentally - perhaps Clone keeps properties when additionalProperties is true?)
This mismatch didn't really surface to me while coding because of the exact property checks in Typescript - so it wasn't immediately obvious to me at the type level. Things typechecked fine, but the implementation wound up unexpected.
Thanks again for the awesome library, I've been using it for quite some time now and it's still my go-to for good reason!
Hi there. Congrats on 1.0 release, I'm excited to migrate all my code 😂 (actually though!)
The codebase I encountered this situation in is still on an older version of typebox, but the typebox code that I tracked it back to seems to still be present in master, so I'm filing this anyway.
--
I encountered something unexpected while working on some code performing OAuth2 duties. Since the spec allows for auth via either headers or body parameters, I created a schema that is essentially "maybe a given type, or maybe a given type plus the two body parameters":
I've also created a type for holding secrets in a weakmap, to avoid accidentally exposing them, and used a transform type to handle wrapping/unwrapping secrets as early/late as possible:
I finally got to a point where I'm doing end-to-end testing against an OAuth server, and was surprised to be getting 4xx errors saying "invalid client secret". After some debugging, I found that the body did not include the unwrapped secret, for the following combination of reasons:
typebox/src/value/transform/encode.ts
Lines 200 to 212 in 5130ee5
{ additionalProperties: false }Setting
additionalPropertiesto false on the object does produce the expected result:(with
additionalProperties: false)(without
additionalProperties: false)... so this isn't exactly a bug, just a fairly sharp corner that I banged my knee on.
While the behavior is explicable and makes sense in light of my discoveries, it might be worth considering different logic here. If I'm explicitly calling "Encode" on some data, I am not expecting it to return unencoded data -- so it's surprising to me that the first loop exists at all. The Encode function also appears to operate on the data in-place, so my output was including the
client_idandclient_secretproperties because they were present on the input, but their content was unexpected because the schema being applied was not the one I expected. (This may have changed in the new version, but I have a lot of work to do before I can verify experimentally - perhaps Clone keeps properties when additionalProperties is true?)This mismatch didn't really surface to me while coding because of the exact property checks in Typescript - so it wasn't immediately obvious to me at the type level. Things typechecked fine, but the implementation wound up unexpected.
Thanks again for the awesome library, I've been using it for quite some time now and it's still my go-to for good reason!