gapic-showcase icon indicating copy to clipboard operation
gapic-showcase copied to clipboard

`Invalid semicolon separator in query` error when enabling numeric enum feature.

Open mpeddada1 opened this issue 2 years ago • 6 comments

The semicolon is no longer a supported separator as described in golang.org/issue/25192

In the Java gapic-generator, the query parameter json;enum-encoding=int is appended to requests when the numeric enum feature is enabled. See HttpJsonServiceStubClassComposer for reference. This results in the following error message from the showcase server:

{"error":{"code":400,"message":"unrecognized request: GET \"/v1beta1/echo:echo?$alt=json;enum-encoding%3Dint\"","details":null,"Body":"","Header":null,"Errors":null}}
2023/01/25 13:18:00 Received POST request matching '/v1beta1/echo:echo': "/v1beta1/echo:echo?$alt=json;enum-encoding%3Dint"
2023/01/25 13:18:00   urlPathParams (expect 0, have 0): map[]
2023/01/25 13:18:00 error in query string: invalid semicolon separator in query
2023/01/25 13:18:00 http: URL query contains semicolon, which is no longer a supported separator; parts of the query may be stripped when parsed; see golang.org/issue/25192

See also related issue in gapic-generator-java: https://github.com/googleapis/gapic-generator-java/issues/1187

Additionally, replacing; with %3b according to the guidelines results in the parameter being double escaped:

2023/01/27 17:35:50 Received POST request matching '/v1beta1/echo:echo': "/v1beta1/echo:echo?$alt=json%253benum-encoding%3Dint"
2023/01/27 17:35:50   urlPathParams (expect 0, have 0): map[]
2023/01/27 17:35:50 error in query string: unhandled value "json%3benum-encoding=int" for system parameter "$alt"

The issue appears to be rooting from url.ParseQuery. According to the function's Go documentation:

Query is expected to be a list of key=value settings separated by ampersands. A setting without an equals sign is interpreted as a key set to an empty value. Settings containing a non-URL-encoded semicolon are considered invalid.

A few open-ended questions to possibly help address this:

  1. Could this be resolved by url encoding the query parameter before calling url.ParseQuery via URLEncoder.encode(queryString)?
  2. Alternatively, is it possible for systemparam.go to accept & as a separator?

Thank you!

cc/ @burkedavison @blakeli0

mpeddada1 avatar Jan 27 '23 22:01 mpeddada1

As per the comment you cite, the semicolon must be escaped (once). To be clear, the ; is not intended as a separator: the full value of the alt query param (once fully decoded) is json;enum-encoding=int. Changing the semicolon is not a trivial change, as the Google servers expect it. (Though at the moment, the semicolon doesn't need to be escaped there, only in Showcase.)

It seems like the Java query string builder is doing some encoding (if you pass the %, it encodes it to a %25), but it is not doing it fully (when passed a ; as part of a value, it should be encoding it to %3b). Surely there must be a way for Java to encode a ; in a query param? I think that would be the right approach to solving this problem.

vchudnov-g avatar Jan 28 '23 01:01 vchudnov-g

Surely there must be a way for Java to encode a ; in a query param? I think that would be the right approach to solving this problem.

We don't encode the URI in the generator itself, we are relying on a shared library to do it. It's possible but it would not a trivial change as well, it also has a big impact as all apiary libraries are using it for URI encoding as well.

In addition, I don't think changing the Java implementation for showcase tests is the right approach to solve this problem. As you mentioned Google servers expect it and the semicolon doesn't need to be escaped there, only in Showcase, I think a more appropriate approach would be matching the showcase server's behavior with the actual Google server's behavior, which accepts unescaped ; in the query params.

blakeli0 avatar Jan 30 '23 16:01 blakeli0

@blakeli0 : Could you clarify which shared library is responsible for the URI encoding?

burkedavison avatar Feb 02 '23 15:02 burkedavison

@blakeli0 : Could you clarify which shared library is responsible for the URI encoding?

google-http-java-client. I didn't get a chance to test it but the encoding logic is probably in UriTemplate, see the unit tests of the class.

blakeli0 avatar Feb 02 '23 15:02 blakeli0

Had to focus on some other work, so I'm only now getting back to this.

@blakeli0 :

We don't encode the URI in the generator itself, we are relying on a shared library to do it.

Can you post-process the encoding before emitting the request? In that case, you can look for raw semicolons the library did not encode and encode them in the generator.

I think a more appropriate approach would be matching the showcase server's behavior with the actual Google server's behavior, which accepts unescaped ; in the query params.

I agree with this principle, but there are three considerations:

  • An unescaped ; use to be a standard query param separator (now deprecated/non-standard, in favor of &)
  • Google servers accept both ; and %3B and do NOT treat ; as a query param separator. (So Showcase is matching one Google behavior.) In this sense the encoding in Java's UriTemplate is doing the same thing, conformant with the current standard: treating the semicolon as a non-special character that does not need to be escaped.
  • Showcase is written in Go, and Go treats un-escaped semicolons as query param separators for legacy reasons: see this issue and this issue. I will see whether we can intercept the query string before it gets decoded.

vchudnov-g avatar Mar 14 '23 00:03 vchudnov-g

Update: I have not had a chance to work on this. It seems to me that there are two fundamental parties at fault here: the Java libraries for not encoding ; in a way that would be acceptable to legacy systems (like Go) that treat ; as intended but not allowed separators; and the Go libraries in the first place for assuming all unescaped ;s are intended as separators when they could simply be part of the values.

On the Go side, as I said, we should see whether we can intercept the query string in Showcase before it gets processed, but I haven't gotten around to it.

On the Java side, is there any way to get the library to escape ; in query string values?

vchudnov-g avatar Jul 18 '24 20:07 vchudnov-g