-
Notifications
You must be signed in to change notification settings - Fork 63
Cross platform test and UTF-8 fixes #1553
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
I noticed all the encoding changes are in |
oh good point: _cli is not used by other modules so it's possible that some of your code has the same issues (maybe it was made following the _cli module here?) |
Indeed, we were also reading and writing signatures without specifying an encoding. Forcing everything to utf-8 seemed to resolve our issues
model-transparency will be thinking about this too |
For the JSON bundle, I think UTF-8 is more conformant depending on how one interprets "closed ecosystem"? https://datatracker.ietf.org/doc/html/rfc8259#section-8.1
|
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Apparently Python really likes to use cp1252 on some platforms but our bundles must not be platform specific. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
To be clear, I didn't mean we should think about changing the representation on disk: utf-8 text is fine --I was just thinking that it might be clearer to read bytes and pass those to the json parser (double checking that it actually uses utf-8 in all cases). |
This should fix #1552
Some details on the impact of this issue: