Tr/fix/issue 512/malformed links (#1045)

* feat: adding hash function to ensure indexes are all lowercase and we can handle upper case collection names fix: fix tests that broke as a result of fix

* update changelog

* fix: applying PR feedback

* tests: PR feedback, adding test that validates two collections with same names but diffeent cases can both be added

* fix: add more robust checking of the bbox for valid coordinate bounds tests: add new tests for more robust tests on bbox check

* docs: update changelog

* fix: adding function to remove 'id' and 'collection' from exclude

* tests: adding tests and fixing issues highlighted by tests

* fix: adding comments for context, docs: updating changelog

Author: theodorehreuter

.nsprc

@@ -14,24 +14,29 @@
         "notes": "fast-xml-parser is a transitive dependency of @aws-sdk/xml-builder and is not directly used by application code. XML parsing from untrusted sources is not part of our usage context.",
         "expiry": "2026-04-23"
     },
-    "1113371": {
+    "1113407": {
         "active": true,
-        "notes": "minimatch is a transitive dependency of @redocly/cli and redoc, used only for API documentation rendering. The vulnerable wildcard pattern matching is not exposed to user input in our usage context, making ReDoS exploitation not feasible.",
+        "notes": "fast-xml-parser is a transitive dependency of @aws-sdk/xml-builder and is not directly used by application code. Entity encoding bypass via regex injection in DOCTYPE is not exploitable as we do not parse untrusted XML input.",
         "expiry": "2026-04-23"
     },
-    "1113398": {
+    "1113428": {
         "active": true,
         "notes": "ajv is a transitive dependency used by webpack build tooling and schema-utils. The $data option ReDoS vulnerability is not exploitable as we do not use ajv directly or pass untrusted data through it.",
-        "expiry": "2026-04-23"
+        "expiry": "2026-04-24"
     },
-    "1113399": {
+    "1113429": {
         "active": true,
         "notes": "ajv is a transitive dependency used by webpack build tooling and schema-utils. The $data option ReDoS vulnerability is not exploitable as we do not use ajv directly or pass untrusted data through it.",
-        "expiry": "2026-04-23"
+        "expiry": "2026-04-24"
     },
-    "1113407": {
+    "1113461": {
         "active": true,
-        "notes": "fast-xml-parser is a transitive dependency of @aws-sdk/xml-builder and is not directly used by application code. Entity encoding bypass via regex injection in DOCTYPE is not exploitable as we do not parse untrusted XML input.",
-        "expiry": "2026-04-23"
+        "notes": "minimatch is a transitive dependency of @redocly/cli and redoc, used only for API documentation rendering. The vulnerable wildcard pattern matching is not exposed to user input in our usage context, making ReDoS exploitation not feasible.",
+        "expiry": "2026-04-25"
+    },
+    "1113466": {
+        "active": true,
+        "notes": "minimatch is a transitive dependency of @redocly/cli and redoc, used only for API documentation rendering. The vulnerable wildcard pattern matching is not exposed to user input in our usage context, making ReDoS exploitation not feasible.",
+        "expiry": "2026-04-25"
     }
 }

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 
 - Added more robust checks on valid/invalid latitude and longitudes for bounding box inputs ([1041](https://github.com/stac-utils/stac-server/pull/1041))
+- Fixed issue where using `fields` extension to `exclude` either `id` or `collection` fields resulted in malformed links with 'undefined' in them due to those fields being required to generate item links.  Those fields, if excluded are now removed after backend opensearch query ([1045](https://github.com/stac-utils/stac-server/pull/1045))
+
 
 ### Added
 

src/lib/api.js

@@ -198,6 +198,32 @@ const validateStartAndEndDatetimes = function (startDateTime, endDateTime) {
   }
 }
 
+/**
+ * ensure that fields necessary for creating links i.e. 'collection' and 'id'
+ * are not excluded from query.  These fields will be removed later to ensure
+ * results match user expectations
+ * @param {Object} fields
+ * @returns {Object}
+ */
+export const createQueryFields = function (fields) {
+  const { exclude } = fields
+  if (exclude) {
+    const filteredExclude = exclude.filter(
+      (field) => field !== 'id' && field !== 'collection'
+    )
+    if (filteredExclude.length === 0) {
+      const { exclude: _removed, ...rest } = fields
+      return rest
+    }
+
+    return {
+      ...fields,
+      exclude: filteredExclude
+    }
+  }
+  return fields
+}
+
 export const extractDatetime = function (params) {
   const { datetime } = params
 
@@ -540,7 +566,6 @@ export const addItemLinks = function (results, endpoint) {
   results.forEach((result) => {
     let { links } = result
     const { id, collection } = result
-
     links = (links === undefined) ? [] : links
     // self link
     links.splice(0, 0, {
@@ -577,6 +602,32 @@ export const addItemLinks = function (results, endpoint) {
   return results
 }
 
+/**
+ * If 'id' or 'collection' were in the 'excluded' fields, they must
+ * be removed.  They were necessary for STAC Item link generation and
+ * can now be removed after link generation if a user wanted to exclude them
+ * Impure, we are potentially mutating 'results'
+ * @param {Object} results
+ * @param {Object} fields - {'exclude': [string], 'include': [string]}
+ * @returns {Object}
+ */
+export const removeSpecialExcludeFields = function (results, fields) {
+  const { exclude } = fields
+  if (!exclude) return results
+
+  const removeId = exclude.includes('id')
+  const removeCollection = exclude.includes('collection')
+
+  // exit early and avoid forEach loop if possible
+  if (!removeId && !removeCollection) return results
+
+  results.forEach((item) => {
+    if (removeId) delete item.id
+    if (removeCollection) delete item.collection
+  })
+  return results
+}
+
 const wrapResponseInFeatureCollection = function (features, links,
   numberMatched, numberReturned, limit) {
   const fc = {
@@ -688,6 +739,7 @@ const searchItems = async function (
     extractRestrictionCql2Filter(parameters, headers)
   )
   const fields = extractFields(parameters)
+  const queryFields = createQueryFields(fields)
   const ids = extractIds(parameters)
   const allowedCollectionIds = extractAllowedCollectionIds(
     parameters,
@@ -704,7 +756,7 @@ const searchItems = async function (
     query,
     filter: combinedFilter,
     sortby,
-    fields,
+    fields: queryFields,
     ids,
     collections,
     next
@@ -797,6 +849,7 @@ const searchItems = async function (
   }
 
   addItemLinks(responseItems, endpoint)
+  removeSpecialExcludeFields(responseItems, fields)
 
   return wrapResponseInFeatureCollection(responseItems, links, numberMatched, numberReturned, limit)
 }

src/lib/database.js

@@ -778,6 +778,7 @@ async function populateUnrestrictedIndices() {
 }
 
 export async function constructSearchParams(parameters, page, limit) {
+  // console.log('DEBUG - parameters %j', parameters)
   const { id, collections, filter } = parameters
 
   let body

tests/system/test-api-search-get.js

@@ -444,3 +444,67 @@ test('/search invalid bbox throws error', async (t) => {
     )
   }
 })
+
+test('GET /search using "exclude" returns properly formatted links', async (t) => {
+  const fixtureFiles = [
+    'LC80100102015050LGN00.json',
+    'LC80100102015082LGN00.json'
+  ]
+  const items = await Promise.all(fixtureFiles.map((x) => loadJson(x)))
+  await processMessages(items)
+  await refreshIndices()
+
+  // specificall testing that using 'collection' and/or 'id' does not return
+  // any 'undefined' links, fields required to generate links
+  {
+    const response = await t.context.api.client.get('search', {
+      resolveBodyOnly: false,
+      searchParams: new URLSearchParams({
+        collections: 'landsat-8-l1',
+        limit: 1,
+        fields: '-assets,-bbox,-stac_version,-collection,-id',
+      })
+    })
+    t.is(response.statusCode, 200)
+    t.is(response.body.features.length, 1)
+
+    // check 'self' link
+    const selfLink = response.body.features[0].links.find((l) => l.rel === 'self')
+    const selfPath = new URL(selfLink.href).pathname
+    t.is(selfPath, '/collections/landsat-8-l1/items/LC80100102015082LGN00')
+
+    const parentLink = response.body.features[0].links.find((l) => l.rel === 'parent')
+    const parentPath = new URL(parentLink.href).pathname
+    t.is(parentPath, '/collections/landsat-8-l1')
+
+    const collectionLink = response.body.features[0].links.find((l) => l.rel === 'collection')
+    const collectionPath = new URL(collectionLink.href).pathname
+    t.is(collectionPath, '/collections/landsat-8-l1')
+  }
+  {
+    // test without excluding required 'id' or 'collection' fields to generate link
+    const response = await t.context.api.client.get('search', {
+      resolveBodyOnly: false,
+      searchParams: new URLSearchParams({
+        collections: 'landsat-8-l1',
+        limit: 1,
+        fields: '-assets,-bbox,-stac_version',
+      })
+    })
+    t.is(response.statusCode, 200)
+    t.is(response.body.features.length, 1)
+
+    // check 'self' link
+    const selfLink = response.body.features[0].links.find((l) => l.rel === 'self')
+    const selfPath = new URL(selfLink.href).pathname
+    t.is(selfPath, '/collections/landsat-8-l1/items/LC80100102015082LGN00')
+
+    const parentLink = response.body.features[0].links.find((l) => l.rel === 'parent')
+    const parentPath = new URL(parentLink.href).pathname
+    t.is(parentPath, '/collections/landsat-8-l1')
+
+    const collectionLink = response.body.features[0].links.find((l) => l.rel === 'collection')
+    const collectionPath = new URL(collectionLink.href).pathname
+    t.is(collectionPath, '/collections/landsat-8-l1')
+  }
+})

tests/system/test-api-search-post.js

@@ -1679,3 +1679,60 @@ test('/search invalid bbox throws error', async (t) => {
     )
   }
 })
+
+test('POST /search using "exclude" returns properly formatted links', async (t) => {
+  {
+    // test by excluding fields required to generate links ('id' and 'collection')
+    const response = await t.context.api.client.post(
+      'search',
+      {
+        resolveBodyOnly: false,
+        json: {
+          collections: ['landsat-8-l1'],
+          limit: 1,
+          fields: { exclude: ['assets', 'bbox', 'stac_version', 'id', 'collection'] }
+        } }
+    )
+    t.is(response.statusCode, 200)
+    t.is(response.body.features.length, 1)
+
+    const selfLink = response.body.features[0].links.find((l) => l.rel === 'self')
+    const selfPath = new URL(selfLink.href).pathname
+    t.is(selfPath, '/collections/landsat-8-l1/items/LC80100102015082LGN00')
+
+    const parentLink = response.body.features[0].links.find((l) => l.rel === 'parent')
+    const parentPath = new URL(parentLink.href).pathname
+    t.is(parentPath, '/collections/landsat-8-l1')
+
+    const collectionLink = response.body.features[0].links.find((l) => l.rel === 'collection')
+    const collectionPath = new URL(collectionLink.href).pathname
+    t.is(collectionPath, '/collections/landsat-8-l1')
+  }
+  {
+    // test without excluding fields required to generate links
+    const response = await t.context.api.client.post(
+      'search',
+      {
+        resolveBodyOnly: false,
+        json: {
+          collections: ['landsat-8-l1'],
+          limit: 1,
+          fields: { exclude: ['assets', 'bbox', 'stac_version'] }
+        } }
+    )
+    t.is(response.statusCode, 200)
+    t.is(response.body.features.length, 1)
+
+    const selfLink = response.body.features[0].links.find((l) => l.rel === 'self')
+    const selfPath = new URL(selfLink.href).pathname
+    t.is(selfPath, '/collections/landsat-8-l1/items/LC80100102015082LGN00')
+
+    const parentLink = response.body.features[0].links.find((l) => l.rel === 'parent')
+    const parentPath = new URL(parentLink.href).pathname
+    t.is(parentPath, '/collections/landsat-8-l1')
+
+    const collectionLink = response.body.features[0].links.find((l) => l.rel === 'collection')
+    const collectionPath = new URL(collectionLink.href).pathname
+    t.is(collectionPath, '/collections/landsat-8-l1')
+  }
+})