Fix async redirect handling regression from FOLLOW_REDIRECTS default#1315
Closed
cursor[bot] wants to merge 1 commit into
Closed
Fix async redirect handling regression from FOLLOW_REDIRECTS default#1315cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
Commit #1278 changed redirect following to default true unless FOLLOW_REDIRECTS is explicitly set. That broke async connectors (e.g. Slack Analytics) which rely on intercepting 3xx responses when FOLLOW_REDIRECTS is not configured. Restore the prior default: do not follow redirects in async processing unless FOLLOW_REDIRECTS is explicitly set. Add a unit test that locks in the behavior. Co-authored-by: Erik Schultink <eschultink@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
Bug and impact
Commit #1278 introduced
FOLLOW_REDIRECTSbut changed the default from!processingContext.getAsync()totrue. Async connectors without an explicitFOLLOW_REDIRECTS=FALSEenv var (Slack Analytics has emptyenvironment_variables) now have the HTTP client follow redirects transparently. The proxy never sees the 3xx + Location header, so the async redirect-handling block inApiDataRequestHandleris bypassed and sanitized data is not written to the async output bucket.Root cause
ApiDataRequestHandlerdefaultedsetFollowRedirects(true)whenFOLLOW_REDIRECTSis unset, replacing the prior behavior that disabled redirect following in async mode.Fix and validation
orElse(!processingContext.getAsync())ProxyConfigPropertyjavadoc to document the async/sync default splitasyncModeDisablesRedirectFollowingByDefaultunit testmvn -pl core -am test -Dtest=ApiDataRequestHandlerTest#asyncModeDisablesRedirectFollowingByDefault,ApiDataRequestHandlerTest#handleShouldFollowRedirectManuallyInAsyncModeChange implications
FOLLOW_REDIRECTSis not explicitly configured