bitflicker64 commented on code in PR #362:
URL:
https://github.com/apache/hugegraph-computer/pull/362#discussion_r3939680521
##########
vermeer/apps/common/hugegraph_tools.go:
##########
@@ -178,7 +178,7 @@ func testServerIsValid(serverAdds []string, hgSpace,
hGraph, username, password
wg.Add(1)
go func(addr string, ctx context.Context, cancel
context.CancelFunc) {
defer wg.Done()
- url :=
fmt.Sprintf("%v/graphspaces/%v/graphs/%v/schema?format=json", addr, hgSpace,
hGraph)
+ url :=
fmt.Sprintf("http://%v/graphspaces/%v/graphs/%v/schema?format=json";, addr,
hgSpace, hGraph)
req, err := http.NewRequest(http.MethodGet, url, nil)
req.SetBasicAuth(username, password)
Review Comment:
🧹 Optional, or a follow-up issue: `req.SetBasicAuth` here runs before the
`err` check on line 184, so a `http.NewRequest` failure panics instead of
returning. Pre-existing, but one line below your change and in the same
function, so it may be cheapest to fix while reworking the address handling for
imbajin's line 181 comment.
`hgSpace`/`hGraph` reach line 181 straight from `SplitHgName` of the task's
`hugegraph_name` param, which splits on `/` and validates nothing else, so any
ASCII control character in a graph name makes `url.Parse` fail and leaves `req`
nil. On go1.23.7:
```
err=parse "http://server:8080/.../graphs/g\n/schema?format=json": net/url:
invalid control character in URL
PANIC on req.SetBasicAuth: runtime error: invalid memory address or nil
pointer dereference
```
This goroutine has no `recover`, so it takes the worker down rather than
failing one probe. `getHugegraphSchema` repeats the ordering at lines 226-228.
Suggested change: move `if err != nil` above `SetBasicAuth` in both.
`getHugegraphSchema` already logs and returns there; this closure has no return
value, so log before the bare `return`.
--
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]
