Copilot commented on code in PR #1505:
URL: https://github.com/apache/pulsar-client-go/pull/1505#discussion_r3285421969
##########
pulsaradmin/pkg/rest/client.go:
##########
@@ -62,6 +62,10 @@ func (c *Client) newRequest(method, path string) (*request,
error) {
User: base.User,
Host: base.Host,
Path: endpoint(base.Path, u.Path),
+ RawPath: endpoint(
+ base.EscapedPath(),
+ u.EscapedPath(),
+ ),
Review Comment:
Setting url.URL.RawPath here preserves escaped slashes (e.g. %2F) in
outbound requests, but it also changes behavior for existing admin endpoints
that currently pass path fragments with leading/trailing slashes into
pulsarClient.endpoint (e.g. "/health", "/configuration/"). Those fragments get
url.PathEscape’d into "%2Fhealth"/"%2Fconfiguration%2F" (see
pulsaradmin/pkg/admin/brokers.go and broker_stats.go usages), and with RawPath
now honored they will be sent literally, likely resulting in incorrect routing
on the broker side (the server will see a segment like "%2Fhealth" instead of
"health"). This PR needs to either normalize inputs (trim leading/trailing "/"
in pulsarClient.endpoint) or update call sites to pass clean segments,
otherwise many existing endpoints may break once RawPath is used.
##########
pulsaradmin/pkg/admin/brokers.go:
##########
@@ -187,7 +188,7 @@ func (b *broker) UpdateDynamicConfiguration(configName,
configValue string) erro
}
func (b *broker) UpdateDynamicConfigurationWithContext(ctx context.Context,
configName, configValue string) error {
- value := fmt.Sprintf("/configuration/%s/%s", configName, configValue)
+ value := fmt.Sprintf("/configuration/%s/%s", configName,
url.PathEscape(configValue))
Review Comment:
Only configValue is path-escaped here; configName is still interpolated
verbatim into the URL path. If configName contains path-reserved characters
(especially '/'), the request path can still be interpreted incorrectly, so the
escaping is incomplete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
