Argument file needs support for multi-char and BOM

[Falco20019]

What version of protobuf and what language are you using?
libprotoc 3.3.0 / Grpc.Tools 2.28.1

What operating system (Linux, Windows, ...) and version?
Windows 10 Build 1803

What did you do?

1. Create test.rsp containing non-ascii:

--plugin=protoc-gen-grpc=ágrpc_csharp_plugin.exe
--grpc_out=grpc_out
test.proto

2. Run protoc.exe @test.rsp

What did you expect to see
protoc running and creating the files.

What did you see instead?
UTF-8 without BOM: --grpc_out: protoc-gen-grpc: Das System kann die angegebene Datei nicht finden.
UTF-8 with BOM: You seem to have passed an empty string as one of the arguments to C:\Users\dejhbk0l\.nuget\packages\grpc.tools\2.28.1\tools\windows_x64\protoc.exe. This is actually sort of hard to do. Congrats. Unfortunately it is not valid input so the program is going to die now.

Anything else we should know about your project / environment
https://github.com/protocolbuffers/protobuf/blame/master/src/google/protobuf/compiler/command_line_interface.cc#L1326 uses std::ifstream instead of std::wifstream and therefore does not allow BOM or wide characters.

This is a minimal example. We stumbled over this in grpc/grpc#17995 as we distribute protoc and the protoc-gen-grpc through NuGet and the default for dotnet nuget locals global-packages is %userprofile%\.nuget\packages\. For some of our users, the username includes non-ascii characters which leads to protoc-gen-grpc containing non-ascii characters and the argument file not being parsable.

[Falco20019]

CC:

• @anandolee because it's protoc
• @acozzette as implementer of the method
• @jtattermusch as related to Grpc.Tools

[jtattermusch]

I'm not sure if making the ExpandArgumentFile function support std:wifstream is feasible (the functions returns a std::string which is then processed by the command_line_interface.cc logic, and the entire logic is written in terms of std::string and it doesn't know about wide characters at all).

protobuf/src/google/protobuf/compiler/command_line_interface.cc

Line 1425 in 3f5fc4d

bool CommandLineInterface::ExpandArgumentFile(

[Falco20019]

Not too deep into C++, but AFAIK this would mean that the whole chain would need to support std::wstring. So this change would need to go a long way.

It's a bit sad to see that UTF-8 is still not the default in 2021 in so many situations (not just here).

[Master-Code-Programmer]

Not supporting UTF-8 in the year 2022 is a complete anachronism. This really must be fixed.
But std::wstring isn't the solution: http://utf8everywhere.org.
Best would be to just keep using std::string, but make it Unicode capable.

[jtattermusch]

The original link to command_line_interface.cc is no longer valid. This seems to be the location it was referring to:

protobuf/src/google/protobuf/compiler/command_line_interface.cc

Line 1429 in 512f42c

std::ifstream file_stream(file.c_str());

[tonydnewell]

BOM doesn't need supporting. Grpc.Tools writes out the argument files as utf-8 without BOM.

Looking at the protobuf code, the fix isn't changing from std::string to std::wstring and using std::wistream in CommandLineInterface::ExpandArgumentFile - in fact I think that is not the correct solution.

Reading each line of the utf-8 encoded file into a std::string is correctly (each line is just a sequence of bytes). It is when the string is used that conversion from utf-8 to std::wstring needs to be done.

io_win32.cc handles conversion from utf8 to std::wstring correctly (in as_windows_path()).

The problem seems to be when calling the plugins in subprocess.cc. Subprocess::Start() uses CreateProcessA instead of CreateProcessW and doesn't do the utf-8 to wstring conversion.

I've tested my own fix locally.