Skip to content

GH Issue 1718: Lookup to a query in a parent folder contaminates the request's query environment container - #8177

Merged
XingY merged 7 commits into
developfrom
fb_issue1718
Oct 19, 2026
Merged

XingY merged 7 commits into
developfrom
fb_issue1718

Conversation

@XingY

@XingY XingY commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Rationale

Fixes GH Issue 1718: resolving a lookup whose target query lives in another folder leaves the request's query environment container pointing at that folder. Compiling a query sets the thread-local environment container to the compiled query's own container and never puts it back, so everything that reads it later in the request resolves against the wrong folder. Custom compare types build their filter clauses from that value, which is how this surfaced in LIMS: a workflow job's sample type tab looked the job up in the parent folder, found nothing, and showed no samples.

Related Pull Requests

Changes

  • Save and restore the query environment container around lookup resolution in the property-descriptor foreign key, so compiling the lookup's target query no longer leaks its container into the rest of the request.
  • Apply the same guard to the query-based foreign key, which is exposed the same way whenever its lookup targets another container.

@XingY
XingY requested a review from labkey-matthewb October 14, 2026 17:33

@labkey-matthewb labkey-matthewb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be the right fix. Unfortunately, I think we have to dig deeper and looks at who is overwriting the environment and why. Originally the query environment was only meant to support some context functions in LabKey SQL (userid, ismemberof, etc). Unfortunately, some developers have discovered this as a way to grab a container out of thin air (dangerous). Can we investigate why resolving a lookup calls setEnvirnment() and see if that can be removed?

@XingY

XingY commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

This can't be the right fix. Unfortunately, I think we have to dig deeper and looks at who is overwriting the environment and why. Originally the query environment was only meant to support some context functions in LabKey SQL (userid, ismemberof, etc). Unfortunately, some developers have discovered this as a way to grab a container out of thin air (dangerous). Can we investigate why resolving a lookup calls setEnvirnment() and see if that can be removed?

QueryService.get().setEnvironment(QueryService.Environment.CONTAINER, getSchema().getContainer()) in QueryDefinition.getTable is the culprit, which been the way since Method.FolderInfo was introduced in 2013. It was added to support labkeyMethod.foldername and labkeyMethod.folderpath, but it doesn't seem right. I've pushed a change to remove that.

at org.labkey.query.sql.Query.getTableInfo(Query.java:613) at org.labkey.query.QueryDefinitionImpl.createTable(QueryDefinitionImpl.java:615) at org.labkey.query.QueryDefinitionImpl.getTable(QueryDefinitionImpl.java:574) at org.labkey.api.query.UserSchema.getTable(UserSchema.java:202) at org.labkey.api.query.UserSchema.getTable(UserSchema.java:169) at org.labkey.api.query.UserSchema.getTable(UserSchema.java:162) at org.labkey.api.query.PdLookupForeignKey.findTableInfo(PdLookupForeignKey.java:215) at org.labkey.api.query.PdLookupForeignKey.getLookupTableInfo(PdLookupForeignKey.java:177) at org.labkey.api.query.PdLookupForeignKey.createLookupColumn(PdLookupForeignKey.java:222) at org.labkey.api.data.BaseColumnInfo.getDisplayField(BaseColumnInfo.java:768) at org.labkey.api.data.DataColumn.getDisplayField(DataColumn.java:186) at org.labkey.api.data.DataColumn.<init>(DataColumn.java:98) at org.labkey.api.data.DataColumn.<init>(DataColumn.java:92) at org.labkey.api.data.ColumnInfo$1.createRenderer(ColumnInfo.java:66) at org.labkey.api.data.BaseColumnInfo.getRenderer(BaseColumnInfo.java:1661) at org.labkey.query.QueryDefinitionImpl.getDisplayColumns(QueryDefinitionImpl.java:1059) at org.labkey.api.query.QueryView.getDisplayColumns(QueryView.java:3037) at org.labkey.api.query.QueryView.configureDataRegion(QueryView.java:2044) at org.labkey.api.query.QueryView.createDataRegion(QueryView.java:2038) at org.labkey.api.query.QueryView.createDataView(QueryView.java:2119) at org.labkey.api.query.QueryView.exportToApiResponse(QueryView.java:2851) at org.labkey.api.action.ApiQueryResponse.<init>(ApiQueryResponse.java:129) at org.labkey.api.action.ExtendedApiQueryResponse.<init>(ExtendedApiQueryResponse.java:88) at org.labkey.api.action.ReportingApiQueryResponse.<init>(ReportingApiQueryResponse.java:48) at org.labkey.query.controllers.QueryController$SelectRowsAction.execute(QueryController.java:3744) at org.labkey.query.controllers.QueryController$SelectRowsAction.execute(QueryController.java:3698)

@XingY
XingY requested a review from labkey-matthewb October 16, 2026 17:01
@XingY
XingY merged commit 0bb3576 into develop Oct 19, 2026
8 checks passed
@XingY
XingY deleted the fb_issue1718 branch October 19, 2026 19:35
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