Le mardi 13 août 2024, 11:54:26 UTC Herwin Weststrate a écrit :
> I've found one possibly breaking change between the current 3.2.1 and
> the proposed 3.2.5: the encoding of binary attributes in JSON. This
> might be a fringe issue.
>
> I have used this configuration:
>
> update request {
> &Class := "0x313233"
> }
> rest
>
> This is put in the post-auth section of the default site. The Class
> attribute is a binary/octets type attribute, and is added to simplify
> reproduction. The rest module has been configured to work with the file
> `src/modules/rlm_rest/demo.pl` of the FreeRADIUS repository (but we only
> need to look at the request, so just listening with netcat on the
> correct port works too). The body type of the rest module is set to
> JSON.
>
> With version 3.2.1+dfsg-4+deb12u1 (bookworm stable), the HTTP request
> looks like this:
>
> "Class":{"type":"octets","value":["0x313233"]}
>
> Version 3.2.5+dfsg-3~deb12u1 does not add this hex conversion, but
> instead uses the textual representation:
>
> "Class":{"type":"octets","value":["123"]}
>
> Non-printable characters are escaped with unicode escaping (I guess
> that's the term?), so "0x01" is transmitted as:
>
> "Class":{"type":"octets","value":["\u0001"]}
>
> This change might break things if the REST backend (which is not part of
> freeradius itself) expects the hex strings. Our backend was dumb enough
> to just strip the first two characters of an octets type attribute
> (without checking if they were equal to "0x") and unescape the rest of
> the string, and that breaks pretty hard.
>
> The change is done in [1] and I'm not sure how to interpret the bug
> report: the second comment say "JSON is not valid", but the JSON string
> in the example is perfectly valid.I think they said that the type does not correspond to the JSON schema, and I agreed with upstream here. Encoding as hex is an error. JSON5 solve the problem by allowing integer to be encoded as hex but no string. > > The change can be reverted by reverting that single line commit linked > in the bug report (I have tested that one). This does keep the behaviour > stable for the Debian bookworm users, but it introduces an > incompatibility with the upstream 3.2.5 version, which can be confusing > when you're reading documentation for the upstream version. I think it is more a bug fix that need maybe a changelog entry and a warning in the DSA. > I'm not sure what my advise here would be. Personally, I would love to > see that change reverted simply because it saves me from some work, but > that's not really a valid reason. The change is incompatible with the > current version, but only in very specific setups, so I'm not sure if > anybody else would be affected. > > [1] https://github.com/FreeRADIUS/freeradius-server/issues/5285 > >
signature.asc
Description: This is a digitally signed message part.
