Fix Location header to respect X-Forwarded-Proto and X-Forwarded-Host headers#3095
Fix Location header to respect X-Forwarded-Proto and X-Forwarded-Host headers#3095
Conversation
… headers Updated SqlMutationEngine.cs and SqlResponseHelpers.cs to use the SqlPaginationUtil.ResolveRequestScheme and ResolveRequestHost methods when constructing Location headers. This ensures that when behind a reverse proxy (like Azure API Management), the Location header uses the original client request scheme (HTTPS) instead of the internal connection scheme (HTTP). Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
|
@copilot the changes look good to me, confirm if nothing else is breaking with this change. |
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
|
@copilot why is this PR not published yet. |
Verified the changes by building the solution - build succeeded with 0 errors and 0 warnings. The changes are minimal and follow the same pattern already used by Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The PR is published and the changes have been pushed to the |
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
|
@copilot "enum": ["L1", "L1L2", null] need to be omitable and default to "L1" |
This comment appears to be about cache level schema changes, but this PR only addresses the Location header X-Forwarded-Proto/Host issue. The files changed are |
1. SqlMutationEngine.cs (Lines 400-404) - Stored Procedure Location HeaderC# Context: Used for INSERT operations on stored procedures (line 397), returns 201 Created with Location header. 2. SqlResponseHelpers.cs (Lines 384-390) - Entity Location HeaderC#
Context: Used in 3. SqlPaginationUtil.cs - Pagination URI MethodsConstructBaseUriForPagination (Lines 588-606)C#
ResolveRequestScheme (Lines 759-775)C#
ResolveRequestHost (Lines 783-799)C#
Key Points: Both helpers support forwarded headers (X-Forwarded-Proto/Host) for proxy scenarios, with validation and fallback to original request values. ⏹️ End subagent: explore Now I have a complete understanding. Let me also check the issue #3032 referenced in the PR to understand the original bug report context. Now I fully understand the situation: PR #3095 (the original, still open): Changed PR #3135 (the mistake, now closed): Proposed a better approach — instead of exposing internal methods, just reuse the existing What the user wants: Convert the PR #3135 approach into recommendations/instructions that can be given to Copilot to apply in PR #3095. No actual code changes in this PR. Now let me understand what this PR's branch should contain. The user wants a recommendations document — something they can provide as instruction to Copilot working on PR #3095. Let me create that. Progress update: Initial plan for converting PR #3135 to recommendations Now let me check if there's an existing PR description or any initial file that was committed from the "Initial plan" commit. Check what was in the initial plan commit The initial plan commit was empty (no file changes). Good. Now I need to create the recommendations. The user wants recommendations they can provide to Copilot working on PR #3095. This should be a file with clear instructions. Let me create this as a clear set of review recommendations that the user can copy-paste as a comment on PR #3095 to instruct Copilot. Create RECOMMENDATIONS-PR-3095.md
Recommendations for PR #3095
|
|
... It looks like this comment may have been cut off or has formatting issues. Could you clarify what you'd like me to change regarding SqlMutationEngine.cs lines 400-404? |
Why make this change?
POST to stored procedure/entity endpoints returns Location header with
http://scheme even when original request is HTTPS. This occurs behind reverse proxies (Azure API Management, Container Apps) where internal traffic uses HTTP but theX-Forwarded-Proto: httpsheader is set.What is this change?
Reuses existing
SqlPaginationUtil.ResolveRequestSchemeandResolveRequestHosthelpers (already used for paginationnextLink) in mutation handlers:private→internalforResolveRequestScheme,ResolveRequestHost,IsValidScheme,IsValidHostBefore:
After:
How was this tested?
Existing validation logic from pagination already handles header validation (scheme must be
http/https, host validated for dangerous characters).Sample Request(s)
Response now correctly includes:
Original prompt
This section details on the original issue you should resolve
<issue_title>[Bug]: POST to rest endpoint on https for stored proc returns a location header with insecure http scheme</issue_title>
<issue_description>### What happened?
A bug happened!
Posting to a stored procedure endpoint returns a location header that doesn't match the request incoming scheme. This results in errors on clients that don't allow insecure redirects.
The returned scheme should match the incoming scheme
Version
1.6.84
What database are you using?
Azure SQL
What hosting model are you using?
Container Apps
Which API approach are you accessing DAB through?
REST
Relevant log output
Code of Conduct
<agent_instructions>Resolve this bug with as little code change as possible.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@JerryNixon ## Root CauseThe issue is in how DAB constructs the
Locationheader for stored procedure POST requests. When building the location header URL, DAB is not properly respecting the incoming request's HTTPS scheme when behind a reverse proxy.The Problem Code
Looking at
SqlMutationEngine.csfor stored procedure handling:The problem is that
httpContext.Request.Schemereturns the scheme of the connection between the reverse proxy (Azure API Management) and the container app (HTTP), not the original client request scheme (HTTPS).Why Regular Entity Inserts Work Better
Interestingly, for regular table/view POST operations, DAB uses
SqlResponseHelpers.ConstructCreatedResultResponse(), which has the same issue:The Solution That Exists for Pagination
DAB already has the correct implementation for handling forwarded headers in the pagination utility (
SqlPaginationUtil.cs):The helper methods check for
X-Forwarded-ProtoandX-Forwarded-Hostheaders: