Skip to content

Commit 242a5ee

Browse files
committed
Fix double-slash URL path bug across 74 GetRequest methods
Replace manual strings.Builder URL path construction with typed path builder structs that reject empty required segments. Fixes an issue where empty path segments produced a double-slash // that http.NewRequest misparsed as an RFC 3986 authority separator. Ref: opensearch-project#804 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 681c571 commit 242a5ee

172 files changed

Lines changed: 2354 additions & 1894 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1515
- Add `Status` field (`json.RawMessage`) to `TasksGetResp`, `TasksListTask`, and `TaskCancelInfo` for polymorphic task status data; add typed status structs matching the OpenSearch API specification: `BulkByScrollTaskStatus`, `ReplicationTaskStatus`, `ResyncTaskStatus`, `PersistentTaskStatus`; add `Parse*` helpers and `BulkByScrollTaskStatusOrException` for sliced task status ([#788](https://github.com/opensearch-project/opensearch-go/issues/788))
1616
- Test parallelization support via TEST_PARALLEL environment variable (default: CPU cores - 1, minimum 1)
1717
- opensearchapi/testutil package with test suite, client helpers, and JSON comparison utilities
18+
- Add typed path segment types and struct-per-shape path builders for compile-time URL construction safety ([#617](https://github.com/opensearch-project/opensearch-go/issues/617), [#650](https://github.com/opensearch-project/opensearch-go/issues/650))
19+
- Domain types: `Index`, `Indices`, `Action`, `DocumentID`, `Alias`, `Repo`, `Snapshot`, `NodeID`, `Plugin`, `Policy`, `Block`, `Prefix`, `Suffix`, `Name`, `Resource`, `Attr`, `Value`, `Metric`, `IndexMetric`, `Metrics`, `NodeFilter`
20+
- 25 path builder structs: `IndexPath`, `DocumentPath`, `IndicesActionPath`, `AliasPath`, `SnapshotPath`, `NodesPath`, `PluginResourcePath`, `PrefixActionPath`, `ActionSuffixPath`, etc.
21+
- `ToIndices([]string)` and `MustBuild()` helpers
22+
- Published API types in `opensearchapi/` and `plugins/` remain `string`/`[]string`; casts to domain types happen internally in `GetRequest()` methods
1823
- opensearchtransport/testutil package with PollUntil helper for eventual consistency testing (ISM policies, index readiness, cluster state changes)
1924
- Configuration option `IncludeDedicatedClusterManagers` for controlling cluster manager node routing ([#765](https://github.com/opensearch-project/opensearch-go/issues/765))
2025
- Policy-based routing system for improved request routing and service availability ([#771](https://github.com/opensearch-project/opensearch-go/pull/771))
@@ -150,6 +155,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
150155

151156
- Fix pool replacement orphaning resurrection goroutines during node discovery, causing connections to become permanently dead with no active health checker ([#786](https://github.com/opensearch-project/opensearch-go/pull/786))
152157
- Extract `newMultiServerPoolFromClientWithLock` as single source of truth for Client-to-pool settings propagation ([#786](https://github.com/opensearch-project/opensearch-go/pull/786))
158+
- Fix URL path construction across 74 `GetRequest` methods where empty path segments produced a double-slash `//` that `http.NewRequest` misparsed as an RFC 3986 authority separator; replace manual `strings.Builder` paths with typed path builder structs that reject empty required segments ([#617](https://github.com/opensearch-project/opensearch-go/issues/617), [#650](https://github.com/opensearch-project/opensearch-go/issues/650))
153159
- Fix connection lifecycle bug in multiServerPool.OnFailure where connections were scheduled for resurrection before being moved from ready to dead list, causing potential race conditions
154160
- Fix flaky connection integration test by replacing arbitrary sleep times with proper server readiness polling
155161
- Fix cluster readiness checks in integration tests to handle HTTPS cold start delays (increase timeout to 15s)

opensearchapi/api_bulk.go

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"context"
1111
"io"
1212
"net/http"
13-
"strings"
1413

1514
"github.com/opensearch-project/opensearch-go/v4"
1615
)
@@ -38,24 +37,11 @@ type BulkReq struct {
3837

3938
// GetRequest returns the *http.Request that gets executed by the client
4039
func (r BulkReq) GetRequest() (*http.Request, error) {
41-
var path strings.Builder
42-
//nolint:mnd // 7 is the max number of static chars
43-
path.Grow(7 + len(r.Index))
44-
45-
if len(r.Index) > 0 {
46-
path.WriteString("/")
47-
path.WriteString(r.Index)
40+
path, err := opensearch.PrefixActionPath{Prefix: opensearch.Prefix(r.Index), Action: "_bulk"}.Build()
41+
if err != nil {
42+
return nil, err
4843
}
49-
50-
path.WriteString("/_bulk")
51-
52-
return opensearch.BuildRequest(
53-
"POST",
54-
path.String(),
55-
r.Body,
56-
r.Params.get(),
57-
r.Header,
58-
)
44+
return opensearch.BuildRequest(http.MethodPost, path, r.Body, r.Params.get(), r.Header)
5945
}
6046

6147
// BulkResp represents the returned struct of the /_bulk response

opensearchapi/api_cat-aliases.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatAliasesReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatAliasesReq) GetRequest() (*http.Request, error) {
25-
aliases := strings.Join(r.Aliases, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/aliases/") + len(aliases))
28-
path.WriteString("/_cat/aliases")
29-
if len(r.Aliases) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(aliases)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/aliases", Suffix: opensearch.Suffix(strings.Join(r.Aliases, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatAliasesResp represents the returned struct of the /_cat/aliases response

opensearchapi/api_cat-allocation.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatAllocationReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatAllocationReq) GetRequest() (*http.Request, error) {
25-
nodes := strings.Join(r.NodeIDs, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/allocation/") + len(nodes))
28-
path.WriteString("/_cat/allocation")
29-
if len(r.NodeIDs) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(nodes)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/allocation", Suffix: opensearch.Suffix(strings.Join(r.NodeIDs, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatAllocationsResp represents the returned struct of the /_cat/allocation response

opensearchapi/api_cat-count.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatCountReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatCountReq) GetRequest() (*http.Request, error) {
25-
indices := strings.Join(r.Indices, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/count/") + len(indices))
28-
path.WriteString("/_cat/count")
29-
if len(r.Indices) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(indices)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/count", Suffix: opensearch.Suffix(strings.Join(r.Indices, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatCountsResp represents the returned struct of the /_cat/count response

opensearchapi/api_cat-fielddata.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatFieldDataReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatFieldDataReq) GetRequest() (*http.Request, error) {
25-
fielddata := strings.Join(r.FieldData, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/fielddata/") + len(fielddata))
28-
path.WriteString("/_cat/fielddata")
29-
if len(r.FieldData) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(fielddata)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/fielddata", Suffix: opensearch.Suffix(strings.Join(r.FieldData, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatFieldDataResp represents the returned struct of the /_cat/fielddata response

opensearchapi/api_cat-indices.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatIndicesReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatIndicesReq) GetRequest() (*http.Request, error) {
25-
indices := strings.Join(r.Indices, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/indices/") + len(indices))
28-
path.WriteString("/_cat/indices")
29-
if len(r.Indices) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(indices)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/indices", Suffix: opensearch.Suffix(strings.Join(r.Indices, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatIndicesResp represents the returned struct of the /_cat/indices response

opensearchapi/api_cat-recovery.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatRecoveryReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatRecoveryReq) GetRequest() (*http.Request, error) {
25-
indices := strings.Join(r.Indices, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/recovery/") + len(indices))
28-
path.WriteString("/_cat/recovery")
29-
if len(r.Indices) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(indices)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/recovery", Suffix: opensearch.Suffix(strings.Join(r.Indices, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatRecoveryResp represents the returned struct of the /_cat/recovery response

opensearchapi/api_cat-segments.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatSegmentsReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatSegmentsReq) GetRequest() (*http.Request, error) {
25-
indices := strings.Join(r.Indices, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/segments/") + len(indices))
28-
path.WriteString("/_cat/segments")
29-
if len(r.Indices) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(indices)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/segments", Suffix: opensearch.Suffix(strings.Join(r.Indices, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatSegmentsResp represents the returned struct of the /_cat/segments response

opensearchapi/api_cat-shards.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,11 @@ type CatShardsReq struct {
2222

2323
// GetRequest returns the *http.Request that gets executed by the client
2424
func (r CatShardsReq) GetRequest() (*http.Request, error) {
25-
indices := strings.Join(r.Indices, ",")
26-
var path strings.Builder
27-
path.Grow(len("/_cat/shards/") + len(indices))
28-
path.WriteString("/_cat/shards")
29-
if len(r.Indices) > 0 {
30-
path.WriteString("/")
31-
path.WriteString(indices)
25+
path, err := opensearch.ActionSuffixPath{Action: "_cat/shards", Suffix: opensearch.Suffix(strings.Join(r.Indices, ","))}.Build()
26+
if err != nil {
27+
return nil, err
3228
}
33-
return opensearch.BuildRequest(
34-
"GET",
35-
path.String(),
36-
nil,
37-
r.Params.get(),
38-
r.Header,
39-
)
29+
return opensearch.BuildRequest(http.MethodGet, path, nil, r.Params.get(), r.Header)
4030
}
4131

4232
// CatShardsResp represents the returned struct of the /_cat/shards response

0 commit comments

Comments
 (0)