Skip to content

Commit 1440c06

Browse files
Thomas DesveauxKN4CK3R
Thomas Desveaux
andcommitted
Fix NuGet Package API for $filter with Id equality (go-gitea#31188)
Fixes issue when running `choco info pkgname` where `pkgname` is also a substring of another package Id. Relates to go-gitea#31168 --- This might fix the issue linked, but I'd like to test it with more choco commands before closing the issue in case I find other problems if that's ok. --------- Co-authored-by: KN4CK3R <[email protected]>
1 parent 0853932 commit 1440c06

File tree

2 files changed

+115
-35
lines changed

2 files changed

+115
-35
lines changed

routers/api/packages/nuget/nuget.go

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,20 +95,34 @@ func FeedCapabilityResource(ctx *context.Context) {
9595
xmlResponse(ctx, http.StatusOK, Metadata)
9696
}
9797

98-
var searchTermExtract = regexp.MustCompile(`'([^']+)'`)
98+
var (
99+
searchTermExtract = regexp.MustCompile(`'([^']+)'`)
100+
searchTermExact = regexp.MustCompile(`\s+eq\s+'`)
101+
)
99102

100-
func getSearchTerm(ctx *context.Context) string {
103+
func getSearchTerm(ctx *context.Context) packages_model.SearchValue {
101104
searchTerm := strings.Trim(ctx.FormTrim("searchTerm"), "'")
102-
if searchTerm == "" {
103-
// $filter contains a query like:
104-
// (((Id ne null) and substringof('microsoft',tolower(Id)))
105-
// We don't support these queries, just extract the search term.
106-
match := searchTermExtract.FindStringSubmatch(ctx.FormTrim("$filter"))
107-
if len(match) == 2 {
108-
searchTerm = strings.TrimSpace(match[1])
105+
if searchTerm != "" {
106+
return packages_model.SearchValue{
107+
Value: searchTerm,
108+
ExactMatch: false,
109+
}
110+
}
111+
112+
// $filter contains a query like:
113+
// (((Id ne null) and substringof('microsoft',tolower(Id)))
114+
// https://www.odata.org/documentation/odata-version-2-0/uri-conventions/ section 4.5
115+
// We don't support these queries, just extract the search term.
116+
filter := ctx.FormTrim("$filter")
117+
match := searchTermExtract.FindStringSubmatch(filter)
118+
if len(match) == 2 {
119+
return packages_model.SearchValue{
120+
Value: strings.TrimSpace(match[1]),
121+
ExactMatch: searchTermExact.MatchString(filter),
109122
}
110123
}
111-
return searchTerm
124+
125+
return packages_model.SearchValue{}
112126
}
113127

114128
// https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs
@@ -117,11 +131,9 @@ func SearchServiceV2(ctx *context.Context) {
117131
paginator := db.NewAbsoluteListOptions(skip, take)
118132

119133
pvs, total, err := packages_model.SearchLatestVersions(ctx, &packages_model.PackageSearchOptions{
120-
OwnerID: ctx.Package.Owner.ID,
121-
Type: packages_model.TypeNuGet,
122-
Name: packages_model.SearchValue{
123-
Value: getSearchTerm(ctx),
124-
},
134+
OwnerID: ctx.Package.Owner.ID,
135+
Type: packages_model.TypeNuGet,
136+
Name: getSearchTerm(ctx),
125137
IsInternal: util.OptionalBoolFalse,
126138
Paginator: paginator,
127139
})
@@ -168,10 +180,8 @@ func SearchServiceV2(ctx *context.Context) {
168180
// http://docs.oasis-open.org/odata/odata/v4.0/errata03/os/complete/part2-url-conventions/odata-v4.0-errata03-os-part2-url-conventions-complete.html#_Toc453752351
169181
func SearchServiceV2Count(ctx *context.Context) {
170182
count, err := nuget_model.CountPackages(ctx, &packages_model.PackageSearchOptions{
171-
OwnerID: ctx.Package.Owner.ID,
172-
Name: packages_model.SearchValue{
173-
Value: getSearchTerm(ctx),
174-
},
183+
OwnerID: ctx.Package.Owner.ID,
184+
Name: getSearchTerm(ctx),
175185
IsInternal: util.OptionalBoolFalse,
176186
})
177187
if err != nil {

tests/integration/api_packages_nuget_test.go

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -401,22 +401,33 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
401401

402402
t.Run("SearchService", func(t *testing.T) {
403403
cases := []struct {
404-
Query string
405-
Skip int
406-
Take int
407-
ExpectedTotal int64
408-
ExpectedResults int
404+
Query string
405+
Skip int
406+
Take int
407+
ExpectedTotal int64
408+
ExpectedResults int
409+
ExpectedExactMatch bool
409410
}{
410-
{"", 0, 0, 1, 1},
411-
{"", 0, 10, 1, 1},
412-
{"gitea", 0, 10, 0, 0},
413-
{"test", 0, 10, 1, 1},
414-
{"test", 1, 10, 1, 0},
411+
{"", 0, 0, 4, 4, false},
412+
{"", 0, 10, 4, 4, false},
413+
{"gitea", 0, 10, 0, 0, false},
414+
{"test", 0, 10, 1, 1, false},
415+
{"test", 1, 10, 1, 0, false},
416+
{"almost.similar", 0, 0, 3, 3, true},
415417
}
416418

417-
req := NewRequestWithBody(t, "PUT", url, createPackage(packageName, "1.0.99"))
418-
req = AddBasicAuthHeader(req, user.Name)
419-
MakeRequest(t, req, http.StatusCreated)
419+
fakePackages := []string{
420+
packageName,
421+
"almost.similar.dependency",
422+
"almost.similar",
423+
"almost.similar.dependant",
424+
}
425+
426+
for _, fakePackageName := range fakePackages {
427+
req := NewRequestWithBody(t, "PUT", url, createPackage(fakePackageName, "1.0.99"))
428+
req = AddBasicAuthHeader(req, user.Name)
429+
MakeRequest(t, req, http.StatusCreated)
430+
}
420431

421432
t.Run("v2", func(t *testing.T) {
422433
t.Run("Search()", func(t *testing.T) {
@@ -463,6 +474,63 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
463474
}
464475
})
465476

477+
t.Run("Packages()", func(t *testing.T) {
478+
defer tests.PrintCurrentTest(t)()
479+
480+
t.Run("substringof", func(t *testing.T) {
481+
defer tests.PrintCurrentTest(t)()
482+
483+
for i, c := range cases {
484+
req := NewRequest(t, "GET", fmt.Sprintf("%s/Packages()?$filter=substringof('%s',tolower(Id))&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
485+
req = AddBasicAuthHeader(req, user.Name)
486+
resp := MakeRequest(t, req, http.StatusOK)
487+
488+
var result FeedResponse
489+
decodeXML(t, resp, &result)
490+
491+
assert.Equal(t, c.ExpectedTotal, result.Count, "case %d: unexpected total hits", i)
492+
assert.Len(t, result.Entries, c.ExpectedResults, "case %d: unexpected result count", i)
493+
494+
req = NewRequest(t, "GET", fmt.Sprintf("%s/Packages()/$count?$filter=substringof('%s',tolower(Id))&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
495+
req = AddBasicAuthHeader(req, user.Name)
496+
resp = MakeRequest(t, req, http.StatusOK)
497+
498+
assert.Equal(t, strconv.FormatInt(c.ExpectedTotal, 10), resp.Body.String(), "case %d: unexpected total hits", i)
499+
}
500+
})
501+
502+
t.Run("IdEq", func(t *testing.T) {
503+
defer tests.PrintCurrentTest(t)()
504+
505+
for i, c := range cases {
506+
if c.Query == "" {
507+
// Ignore the `tolower(Id) eq ''` as it's unlikely to happen
508+
continue
509+
}
510+
req := NewRequest(t, "GET", fmt.Sprintf("%s/Packages()?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
511+
req = AddBasicAuthHeader(req, user.Name)
512+
resp := MakeRequest(t, req, http.StatusOK)
513+
514+
var result FeedResponse
515+
decodeXML(t, resp, &result)
516+
517+
expectedCount := 0
518+
if c.ExpectedExactMatch {
519+
expectedCount = 1
520+
}
521+
522+
assert.Equal(t, int64(expectedCount), result.Count, "case %d: unexpected total hits", i)
523+
assert.Len(t, result.Entries, expectedCount, "case %d: unexpected result count", i)
524+
525+
req = NewRequest(t, "GET", fmt.Sprintf("%s/Packages()/$count?$filter=(tolower(Id) eq '%s')&$skip=%d&$top=%d", url, c.Query, c.Skip, c.Take))
526+
req = AddBasicAuthHeader(req, user.Name)
527+
resp = MakeRequest(t, req, http.StatusOK)
528+
529+
assert.Equal(t, strconv.FormatInt(int64(expectedCount), 10), resp.Body.String(), "case %d: unexpected total hits", i)
530+
}
531+
})
532+
})
533+
466534
t.Run("Next", func(t *testing.T) {
467535
req := NewRequest(t, "GET", fmt.Sprintf("%s/Search()?searchTerm='test'&$skip=0&$top=1", url))
468536
req = AddBasicAuthHeader(req, user.Name)
@@ -520,9 +588,11 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`)
520588
})
521589
})
522590

523-
req = NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, packageName, "1.0.99"))
524-
req = AddBasicAuthHeader(req, user.Name)
525-
MakeRequest(t, req, http.StatusNoContent)
591+
for _, fakePackageName := range fakePackages {
592+
req := NewRequest(t, "DELETE", fmt.Sprintf("%s/%s/%s", url, fakePackageName, "1.0.99"))
593+
req = AddBasicAuthHeader(req, user.Name)
594+
MakeRequest(t, req, http.StatusNoContent)
595+
}
526596
})
527597

528598
t.Run("RegistrationService", func(t *testing.T) {

0 commit comments

Comments
 (0)