Skip to content

TEZ-4742: AMWebController CORS: only reflect Origin when it matches configured Tez UI base URL - #528

Merged
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4742
Jul 21, 2026
Merged

TEZ-4742: AMWebController CORS: only reflect Origin when it matches configured Tez UI base URL#528
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4742

Conversation

@Aggarwal-Raghav

@Aggarwal-RaghavAggarwal-Raghav commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@tez-yetus

tez-yetus commented Jul 19, 2026

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 12sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall4m 40smaster passed
+1 💚compile4m 6smaster passed
+1 💚checkstyle0m 44smaster passed
+1 💚javadoc0m 31smaster passed
+0 🆗spotbugs1m 41stez-dag in master has 537 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall3m 48sthe patch passed
+1 💚codespell1m 36sNo new issues.
+1 💚compile4m 0sthe patch passed
+1 💚javac4m 1sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle0m 37sthe patch passed
+1 💚javadoc0m 30sthe patch passed
+1 💚spotbugs1m 56sthe patch passed
_ Other Tests _
-1 ❌unit76m 4s/patch-unit-root.txtroot in the patch passed.
+1 💚asflicense0m 34sThe patch does not generate ASF License warnings.
102m 39s
ReasonTests
Failed junit teststez.dag.app.web.TestAMWebController
SubsystemReport/Notes
DockerClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/1/artifact/out/Dockerfile
Optional Testsdupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
unameLinux 574a98fe61df 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitytez-personality.sh
git revisionmaster / b9d8c0f
Default JavaEclipse Adoptium-21.0.11+10-LTS
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/1/testReport/
Max. process+thread count2128 (vs. ulimit of 5500)
modulesC: tez-dag U: tez-dag
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/1/console
versionsgit=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@tez-yetus

tez-yetus commented Jul 19, 2026

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 12sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 1 new or modified test files.
_ master Compile Tests _
+1 💚mvninstall4m 48smaster passed
+1 💚compile4m 3smaster passed
+1 💚checkstyle0m 43smaster passed
+1 💚javadoc0m 31smaster passed
+0 🆗spotbugs1m 44stez-dag in master has 537 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall3m 42sthe patch passed
+1 💚codespell1m 36sNo new issues.
+1 💚compile4m 6sthe patch passed
+1 💚javac4m 6sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle0m 37sthe patch passed
+1 💚javadoc0m 29sthe patch passed
+1 💚spotbugs1m 55sthe patch passed
_ Other Tests _
+1 💚unit74m 47sroot in the patch passed.
+1 💚asflicense0m 34sThe patch does not generate ASF License warnings.
101m 26s
SubsystemReport/Notes
DockerClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/2/artifact/out/Dockerfile
Optional Testsdupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
unameLinux e8849ff190eb 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitytez-personality.sh
git revisionmaster / b9d8c0f
Default JavaEclipse Adoptium-21.0.11+10-LTS
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/2/testReport/
Max. process+thread count1535 (vs. ulimit of 5500)
modulesC: tez-dag U: tez-dag
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/2/console
versionsgit=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@Aggarwal-Raghav

Aggarwal-Raghav commented Jul 20, 2026

Copy link
Copy Markdown
ContributorAuthor

CC @abstractdog , ready for review

abstractdog
abstractdog requested changes Jul 21, 2026
View reviewed changes

@abstractdogabstractdog 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.

minor comment

Comment threadtez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java
Comment on lines +140 to 147
if (!historyUrlBase.isEmpty()) {
try {
URL url = URI.create(historyUrlBase).toURL();
origin = url.getProtocol() + "://" + url.getAuthority();
trustedOrigin = url.getProtocol() + "://" + url.getAuthority();
} catch (IllegalArgumentException | MalformedURLException e) {
LOG.debug("Invalid url set for tez history url base: {}", historyUrlBase, e);
}
}

@abstractdogabstractdogJul 21, 2026

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.

maybe refactor this to a method that derives the origin or returns null

abstractdog
abstractdog reviewed Jul 21, 2026
View reviewed changes
Comment threadtez-dag/src/main/java/org/apache/tez/dag/app/web/AMWebController.java
Comment on lines +152 to +153
if (requestOrigin == null || requestOrigin.equals(trustedOrigin)) {
res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, encodeHeader(trustedOrigin));

@abstractdogabstractdogJul 21, 2026•
edited
Loading

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.

make a code comment above this for "educational purposes" regarding why we cannot reflect the request origin without checking the server's trusted origin, as this is the core of this fix

@tez-yetus

tez-yetus commented Jul 21, 2026

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 12sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 1 new or modified test files.
_ master Compile Tests _
+1 💚mvninstall4m 51smaster passed
+1 💚compile4m 3smaster passed
+1 💚checkstyle0m 44smaster passed
+1 💚javadoc0m 31smaster passed
+0 🆗spotbugs1m 44stez-dag in master has 537 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall3m 46sthe patch passed
+1 💚codespell1m 34sNo new issues.
+1 💚compile4m 6sthe patch passed
+1 💚javac4m 6sthe patch passed
+1 💚blanks0m 1sThe patch has no blanks issues.
+1 💚checkstyle0m 37sthe patch passed
+1 💚javadoc0m 29sthe patch passed
+1 💚spotbugs1m 54sthe patch passed
_ Other Tests _
+1 💚unit76m 52sroot in the patch passed.
+1 💚asflicense0m 35sThe patch does not generate ASF License warnings.
103m 35s
SubsystemReport/Notes
DockerClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/3/artifact/out/Dockerfile
Optional Testsdupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
unameLinux a838b4f77acc 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitytez-personality.sh
git revisionmaster / 0f389d2
Default JavaEclipse Adoptium-21.0.11+10-LTS
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/3/testReport/
Max. process+thread count1406 (vs. ulimit of 5500)
modulesC: tez-dag U: tez-dag
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-528/3/console
versionsgit=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

abstractdog
abstractdog approved these changes Jul 21, 2026
View reviewed changes

@abstractdogabstractdog 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.

LGTM

@abstractdog
abstractdog merged commit 74bb821 into apache:masterJul 21, 2026
6 checks passed
asf-gitbox-commits pushed a commit that referenced this pull request Aug 10, 2026
@Aggarwal-Raghav@abstractdog
TEZ-4742: AMWebController CORS: only reflect Origin when it matches c…
bbd9f62
…onfigured Tez UI base URL (#528) (Raghav Aggarwal reviewed by Laszlo Bodor)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abstractdogabstractdogabstractdog approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Aggarwal-Raghav@tez-yetus@abstractdog