[🐛 BUG]: marshal_payload: string field contains invalid UTF-8

[GityaMan]

No duplicates 🥲.

• I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

Updating to 2024.1 breaks some http requests that worked previously.

Version (rr --version)

2024.1+ (protobuf)

How to reproduce the issue?

Send a request with some invalid data. For example, an invalid third-party cookie. We get 500 status code.

curl --request GET \
  --url http://localhost/ \
  --header 'Cookie: name=%A7 '

Relevant log output

No response

[rustatian]

Hey @GityaMan 👋🏻

1. Please, try to update to the latest (2024.3.x) version.
2. Attach a relevant logs (debug).

[GityaMan]

I am using the latest version: 2024.3.1
rr version 2024.3.1 (build time: 2024-12-20T02:15:22+0000, go1.23.4), OS: linux, arch: amd64

[rustatian]

Good, please, attach relevant debug logs as well and check that you use latest versions of the relevant php packages (worker, http, etc).
And last, but not least, what behavior did you expect?

[GityaMan]

{"L":"ERROR","T":"2025-01-09T06:59:21+0000","N":"http ","M":"payload forming error","start":"2025-01-09T06:59:21+0000","elapsed":0,"error":"marshal_payload: string field contains invalid UTF-8"}

{"L":"INFO","T":"2025-01-09T06:59:21+0000","N":"http ","M":"http log","status":500,"method":"GET","URI":"/buy-vsgradlutvhspacksbundleyearmac:1","URL":"/buy-vsgradlutvhspacksbundleyearmac:1","remote_address":"10.56.3.210:45184","read_bytes":0,"write_bytes":0,"start":"2025-01-09T06:59:21+0000","elapsed":0}

[GityaMan]

And last, but not least, what behavior did you expect?

I expect the behavior to be the same as before using protobuf, i.e. the request will be processed.

--

nginx / php-fpm request is also processed correctly.
In version 2023.3.12 the behavior is the same as in nginx/php-fpm

[rustatian]

I'm not sure we should support non-utf-8 symbols. What is the use-case? UTF-8 nowadays is like 99.9% of used characters set.

[GityaMan]

Ignoring such data will work for me.
In the current situation, due to a third-party cookie of some analytical service, all my requests are broken.

[GityaMan]

The response code 400 is more appropriate for the current situation in my opinion.
But I would still like to process such requests.

[rustatian]

Let me check how PHP is compatible with the string/bytes protobuf types (how they're represented in PHP). Because to support the non-utf-8 characters we need to send bytes to PHP, not strings.

[GityaMan]

https://github.com/protocolbuffers/protobuf/tree/main/php%2Fsrc%2FGoogle%2FProtobuf

There are several implementations of prootbuf in php, both in PHP and as extensions written in C. Need to check something specific?

[rustatian]

Yeah, I know. Currently, cookies are represented as a hashmap in our protocol. As you may see, we use strings. Strings are UTF-8 only (this is why you see that error). To support non-utf-8 strings, cookies should be represented as bytes arrays. I guess (I'm not a PHP dev) that in PHP, there is no difference between these wire proto-types (how they're represented in PHP) and that won't require an update from the PHP side.

[GityaMan]

Hey, @rustatian .
Can I expect such http requests to be supported in future versions? Or will more time be needed to investigate this issue?

[rustatian]

Hey @GityaMan 👋🏻
I'll sync with the our PHP team today to double check the case I wrote above. If no BC should be introduced to fix that, then sure, I'll release a patch soon.

[rustatian]

Ok, so, yeah, bytes/string is a single string type in PHP when compiled from protobuf.

[GityaMan]

{"L":"ERROR","T":"2025-01-17T02:49:18+0000","N":"http ","M":"execute","start":"2025-01-17T02:49:18+0000","elapsed":243,"error":"sync_worker_receive_frame: Network: EOF"} 2025-01-17T02:49:18.535892616Z {"L":"INFO","T":"2025-01-17T02:49:18+0000","N":"http ","M":"http log","status":500,"method":"GET","URI":"/","URL":"/","remote_address":"172.19.0.1:46906","read_bytes":0,"write_bytes":40,"start":"2025-01-17T02:49:18+0000","elapsed":243}
Now I get a different error but the response code is still the same - 500.

vendor/spiral/roadrunner-http/src/HttpWorker.php:71 $message->mergeFromString($payload->header) "Error occurred during parsing"
It seems that the DTO was not changed to "bytes" type.

[rustatian]

DTO was changed, wait for the PHP part release.

[rustatian]

You need to update composer deps, especially roadrunner-api-dto.

[GityaMan]

Thank you, now everything works!