field "cc.arduino.cli.commands.Library.Maintainer" contains invalid UTF-8

JAndrassy commented 5 years ago

Describe the problem

Library Manager doesn't show libraries. Output console shows [INFO] ERROR: 2019/10/21 21:46:10 grpc: server failed to encode response: rpc error: code = Internal desc = grpc: error while marshaling: proto: field "cc.arduino.cli.commands.Library.Maintainer" contains invalid UTF-8

Operating system

Linux
Windows

Operating system version

Linux Mint 18.3
Windows 11

Additional context

Additional reports

JAndrassy commented 4 years ago

can this issue be migrated to the arduino-cli repository?

spoenemann commented 4 years ago

Do you have the arduino-cli already installed? If yes, which version? (Try arduino-cli version in a terminal)

JAndrassy commented 4 years ago

I didn't install arduino-cli. Doesn't v0.0.3-alpha.preview have cli bundled?

spoenemann commented 4 years ago

Yes, but if it's found in the global PATH, that version is taken (see arduino/arduino-pro-ide#142).

JAndrassy commented 4 years ago

duro@nuc ~ $ arduino-cli version
arduino-cli: command not found

duro@nuc ~/Arduino Pro IDE-v0.0.3-linux $ ./arduino-pro-ide 
root WARN please install @theia/electron@0.14.0-next.0159cd5b as a runtime dependency
root INFO Theia app listening on http://localhost:42569.
daemon INFO >>> Starting arduino-cli version: 0.6.0 commit: 2049a7a daemon from /home/duro/Arduino Pro IDE-v0.0.3-linux/resources/app/node_modules/arduino-ide-extension/build/arduino-cli...

per1234 commented 3 years ago

Hi @jandrassy. Thanks so much for taking the time to report this!

Would you mind downloading the new 0.1.4 release of Arduino Pro IDE or and letting us know whether the bug still occurs?

From the message, I suspect that the issue is specific to the contents of one of the package index files provided by the Boards Manager URLs. Do you remember if you were using any custom URLs, or did you just have the default configuration, which only provides Boards Manager with the official Arduino package index?

JAndrassy commented 3 years ago

Hi @jandrassy. Thanks so much for taking the time to report this!

Would you mind downloading the new 0.1.4 release of Arduino Pro IDE or and letting us know whether the bug still occurs?

From the message, I suspect that the issue is specific to the contents of one of the package index files provided by the Boards Manager URLs. Do you remember if you were using any custom URLs, or did you just have the default configuration, which only provides Boards Manager with the official Arduino package index?

sorry. Per. I don't have time now for this. the problem then was my name in my published library. I modified library.properties for next version, so now library.properties of my libraries doesn't have UTF character

per1234 commented 3 years ago

Thats enough for me (and yeah, I'm dumb for saying it was a package index). I found the culprit here (ANSI encoding): https://github.com/jandrassy/arduino-library-wifilink/blob/45dda6563bcf9d5c35015beef93054cc1bf8af7a/library.properties#L4

maintainer=Juraj Andrássy <juraj.andrassy@gmail.com>

Steps to reproduce:

Run this command from your Arduino libraries folder (or download the equivalent zip archive and install if you prefer):

git clone https://github.com/jandrassy/arduino-library-wifilink && cd arduino-library-wifilink && git checkout 45dda6563bcf9d5c35015beef93054cc1bf8af7a

Compile a sketch that uses the library:

#include <WiFiLink.h>
void setup() {}
void loop() {}

The compilation process fails:

Sketch uses 2182 bytes (0%) of program storage space. Maximum is 253952 bytes.
Global variables use 226 bytes (2%) of dynamic memory, leaving 7966 bytes for local variables. Maximum is 8192 bytes.
Compilation error: Error: 13 INTERNAL: grpc: error while marshaling: string field contains invalid UTF-8

You can see that the code actually compiled just fine, but this error will block uploading.

The issue does not occur when doing the same with the classic Arduino IDE or Arduino CLI.

ubidefeo commented 3 years ago

@per1234 what's your advice on this? should we force CLI to accept other encodings? If so, let's turn it into a task

per1234 commented 3 years ago

In order to fully form an opinion, I would need to understand two things:

How much complexity would need to be added to the code to support them
What are the use cases where non-UTF-8 encodings are beneficial

If it can be done without significantly harming the maintainability of the code, I'm all for it. Even if we document a UTF-8 requirement in the specifications, people are still going to end up inadvertently using other encodings, so there will always be a benefit to supporting them. But if this is something difficult to support, then I would want to be able to understand better the cost vs benefit balance.

Maybe @silvanocerza or @kittaakos would be able to provide some insight into what would we needed to support other encodings.

Maybe @jandrassy would be able to provide some insight into the use case for non-UTF-8 encoding.

JAndrassy commented 3 years ago

Normally a .properties file should be ISO 8859-1 encoding (Latin-1) and non-Latin-1 characters should be entered by using Unicode escape characters. I use Eclipse IDE and it automatically uses 8859-1 for .properties files.

kittaakos commented 3 years ago

People suggest using bytes instead of string if the data may contain non-UTF-8 characters: https://github.com/protocolbuffers/protobuf/issues/4970#issuecomment-408466702

What do you think @cmaglie?

cmaglie commented 3 years ago

Looking at this article https://en.wikipedia.org/wiki/.properties I see that, starting from Java 9, the default encoding has been changed to UTF-8

In Java 9 and newer, the default encoding for .properties files is UTF-8, and if an invalid UTF-8 byte sequence is encountered it falls back to ISO-8859-1.

maybe we can implement a fallback to ISO-8859-1 in the properties parser. BTW if this turns out to be too much work I would just stick to UTF-8.

kittaakos commented 3 years ago

maybe we can implement a fallback to ISO-8859-1 in the properties parser. BTW if this turns out to be too much work I would just stick to UTF-8.

It's not about the properties parser, but the fact that the maintainer field is string:

https://github.com/arduino/arduino-cli/blob/b2b9fba7ea7ba4cbfb787e469c0c62261d84e2b4/rpc/commands/lib.proto#L142-L143

So no matter what you do with the parser on the CLI side, non-UTF-8 chars won't go through the wire if the gRPC property is string and not bytes.

From the docs:

A string must always contain UTF-8 encoded or 7-bit ASCII text.

cmaglie commented 3 years ago

Yes, what I mean is that we must check that the string is UTF8 before sending it (in this case the input comes from a library.properties file so IMHO we should improve the checks there).

Otherwise, what's the point of sending a malformed UTF-8? you won't have any chance to display it correctly since you don't know the encoding...

cmaglie commented 3 years ago

In the arduino-cli to read the libary.properties we use the parser here -> https://github.com/arduino/go-properties-orderedmap/blob/master/properties.go#L177

I did not investigate how to do UTF-8 sanity checks in golang, BTW fixing this function (to reject non-UFT8 or to fallback to ISO-8859-1 as for java properties files) would automatically solve this issue. We should probably move this issue to the arduino-cli.

EliasA97 commented 3 years ago

Unfortunatelly I'm not that experienced to provide any help. The only thing I found is that from the 0.1.3 version of the Pro IDE and after, it doesn't work, so I still use the Pro 0.1.2 version as it's ways better from the standard IDE. I hope this bug will get fixed in the future!

EliasA97 commented 2 years ago

Any updates about this issue? It still exists on the latest nightly version of the ide

ubidefeo commented 2 years ago

@per1234, as per @cmaglie 's comment

We should probably move this issue to the arduino-cli.

if this is the best thing to do, meaning addressing some sort of filtering/transformation on the CLI side, let's create an issue there

arduino / arduino-cli