Skip to content

Fix: Prevent duplicate root_path in pagination links #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fabito
Copy link

@fabito fabito commented Jun 14, 2025

When a custom ROOT_PATH environment variable is used, the "next" pagination link could incorrectly include the root_path twice.

This was because the BaseLinks.url property, which generates the base for these links, did not properly account for the ROOT_PATH already being part of self.base_url (derived from get_base_url) when joining it with the request path.

The BaseLinks.url property in stac_fastapi/pgstac/models/links.py has been updated to ensure that the request path is made relative to the base_url's path component before joining. This prevents the duplication.

A new test case, test_pagination_link_with_root_path, has been added to tests/api/test_links.py to specifically verify that pagination links are generated correctly when ROOT_PATH is set.

PR Checklist:

  • pre-commit hooks pass locally
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

When a custom `ROOT_PATH` environment variable is used, the "next"
pagination link could incorrectly include the root_path twice.

This was because the `BaseLinks.url` property, which generates the
base for these links, did not properly account for the `ROOT_PATH`
already being part of `self.base_url` (derived from `get_base_url`)
when joining it with the request path.

The `BaseLinks.url` property in `stac_fastapi/pgstac/models/links.py`
has been updated to ensure that the request path is made relative
to the `base_url`'s path component before joining. This prevents
the duplication.

A new test case, `test_pagination_link_with_root_path`, has been
added to `tests/api/test_links.py` to specifically verify that
pagination links are generated correctly when `ROOT_PATH` is set.
@vincentsarago
Copy link
Member

🤔 one more PR/issue about root-path 😭

we extensively worked on this recently #221 which seemed to fix most issues 😭

From experience, issues are usually do to miss configuration on the proxy side

@fabito fabito closed this Jun 16, 2025
@fabito fabito changed the title Fix: Prevent duplicate root_path in pagination links Fix: Prevent duplicate root_path in pagination links Jul 10, 2025
@fabito fabito reopened this Jul 10, 2025
@fabito fabito marked this pull request as ready for review July 10, 2025 10:27
@vincentsarago
Copy link
Member

as much as I would like to fix all the issues in stac-fastapi, I'm still lacking replicable example of this.

we do have k8s/aws lambda/azure deployments set up and the root-path duplication is usually found for mis configuration.

can you tell a bit more about your environment?

@fabito
Copy link
Author

fabito commented Jul 10, 2025

stac-fastapi-pgstac=5.0.2
AWS Lambda runtime
python 3.11

  environment = {
    name         = "My STAC API"
    version      = var.api_version
    cors_origins = "*"
    cachecontrol = "public, max-age=3600"
    debug        = "true"

    POSTGRES_USER        = local.database_username
    POSTGRES_PASS        = local.database_passwprd
    POSTGRES_DBNAME      = local.database_name
    POSTGRES_HOST_READER = local.database_hostname
    POSTGRES_HOST_WRITER = local.database_hostname
    POSTGRES_PORT        = local.database_port
    ROOT_PATH            = "/${local.stac_api_root_path}"
  }

I am not using the existing StacApi instance from stac_fastapi.pgstac.app I am using a custom one that uses a custom PostgresSettings that fetches db password from secret manager.

I will try stac_fastapi.pgstac.app.app, maybe there is something wrong in how I'm creating and configuring the StacApi.

@fabito
Copy link
Author

fabito commented Jul 10, 2025

I just tried with the existing Mangum handler: stac_fastapi.pgstac.app.handler, same problem, unfortunately: next link (in the search response) broken with duplicated root_path.

@vincentsarago
Copy link
Member

I've created https://github.com/developmentseed/stac-fastapi-root-path-lambda to try to replicate and I can't get the duplicate paths

@fabito
Copy link
Author

fabito commented Jul 12, 2025

I think I was able to reproduce the error in these tests:

Let me know if the test fixtures make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants