Copilot commented on code in PR #148: URL: https://github.com/apache/dubbo-go-pixiu-samples/pull/148#discussion_r3294011712
########## https/request.sh: ########## @@ -0,0 +1,33 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +DOMAIN="sample.domain.com" +PORT="8443" +ADDRESS="https://${DOMAIN}:${PORT}"; +RESOLVE="${DOMAIN}:${PORT}:127.0.0.1" + +API1=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=GetUserByName" -d '{"types":"string","values":"tc"}' --header 'Content-Type: application/json') +API2=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=UpdateUserByName" -d '{"types":"string, object","values":["tc",{"id":"0001","code":1,"name":"tc","age":15}]}' --header "Content-Type: application/json") +API3=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=GetUserByCode" -d '{"types":"int","values":1}' --header "Content-Type: application/json") + +ARRAY_API=(${API1} ${API2} ${API3}) + +for element in ${ARRAY_API[@]} +do +echo ${element} Review Comment: `ARRAY_API=(${API1} ${API2} ${API3})` and the subsequent loop expand variables unquoted, so any spaces/newlines in responses (or in variables) will be word-split and echoed as multiple elements. This makes the script output unreliable. Prefer printing each response directly, or use a properly quoted array/loop (e.g., store each response as an element without word-splitting and iterate with quoted expansions). ########## https/request.sh: ########## @@ -0,0 +1,33 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +DOMAIN="sample.domain.com" +PORT="8443" +ADDRESS="https://${DOMAIN}:${PORT}"; +RESOLVE="${DOMAIN}:${PORT}:127.0.0.1" + +API1=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=GetUserByName" -d '{"types":"string","values":"tc"}' --header 'Content-Type: application/json') +API2=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=UpdateUserByName" -d '{"types":"string, object","values":["tc",{"id":"0001","code":1,"name":"tc","age":15}]}' --header "Content-Type: application/json") +API3=$(curl -k --resolve "${RESOLVE}" -s -X POST "${ADDRESS}/api/v1/test-dubbo/com.dubbogo.pixiu.UserService?group=test&version=1.0.0&method=GetUserByCode" -d '{"types":"int","values":1}' --header "Content-Type: application/json") Review Comment: The JSON payload for UpdateUserByName uses `"types":"string, object"` (note the space after the comma). Other samples/tests in this repo use `string,object` without spaces, and the extra whitespace can cause Pixiu/Dubbo type parsing to fail if the implementation does not trim tokens. Align this payload with the format used elsewhere (no spaces) to keep the request script verifiable. ########## https/test/pixiu_test.go: ########## @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package test + +import ( + "context" + "crypto/tls" + "io" + "net" + "net/http" + "strings" + "testing" + "time" +) + +import ( + "github.com/stretchr/testify/assert" +) + +const ( + httpsDomain = "sample.domain.com" + httpsPort = "8443" + httpsTarget = "127.0.0.1:" + httpsPort + httpsAPI = "https://"; + httpsDomain + ":" + httpsPort + "/api/v1/test-dubbo/com.dubbogo.pixiu.UserService" + waitTimeout = 180 * time.Second +) + +func newHTTPSClient() *http.Client { + dialer := &net.Dialer{Timeout: 5 * time.Second} + return &http.Client{ + Timeout: 5 * time.Second, + Transport: &http.Transport{ + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if addr == httpsDomain+":"+httpsPort { + addr = httpsTarget + } + return dialer.DialContext(ctx, network, addr) + }, + TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + ServerName: httpsDomain, + InsecureSkipVerify: true, //nolint:gosec + }, + }, + } +} + +func post(t *testing.T, url string, data string) string { + t.Helper() + + client := newHTTPSClient() + deadline := time.Now().Add(waitTimeout) + var lastBody string + var lastErr error + var lastStatus int + + for { + req, err := http.NewRequest("POST", url, strings.NewReader(data)) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/json") + + resp, err := client.Do(req) + if err == nil && resp != nil { + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + lastBody = string(body) + lastStatus = resp.StatusCode + if resp.StatusCode == http.StatusOK { + return lastBody + } + } else { + lastErr = err Review Comment: In the retry loop, when `client.Do(req)` returns a non-nil `resp` together with a non-nil `err` (which net/http can do in some cases), the current code goes into the `else` branch and does not close `resp.Body`. This can leak connections and make retries behave poorly. Handle/close `resp.Body` whenever `resp != nil`, even if `err != nil` (and consider capturing status/body for diagnostics in that path too). -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
