Skip to content
New issue

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

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

Already on GitHub? # to your account

[CI] Fix xharness local run. #22236

Merged
merged 2 commits into from
Feb 24, 2025
Merged

[CI] Fix xharness local run. #22236

merged 2 commits into from
Feb 24, 2025

Conversation

mandel-macaque
Copy link
Member

In PR #22154 in an attempt to fix some CodeQL warnings we broke the local execution of xharness.

The reason is the following, CodeQL complained about sanitizing the local path and in an attempt to remove the warning a test to make sure that the local path does not have a '/' was added.

The issue is that our requests, when we run xharness locally, will return localpaths of the form: '/20250221_181807/index.html' that is because the request is for 'http://localhost:51234/20250221_181807/index.html'.

In webapps, the path of the request ALWAYS starts with '/' even when the request is for 'http://localhost:51234' (which returns '/').

In an attemp to fix xharness AND keep CodeQL happy I have made the following changes:

  1. Ensure that the full path is one of the allowed ones.
  2. If the path is not one of the allowed ones, check if it is and allowed file.
  3. Do not try to check for '/' or '' but verify that the path instead get the path directory and ensure that is the one for the logs and no other.

This should fix xharness and keep CodeQL happy.

In PR #22154 in an attempt to fix
some CodeQL warnings we broke the local execution of xharness.

The reason is the following, CodeQL complained about sanitizing the
local path and in an attempt to remove the warning a test to make sure that the
local path does not have a '/' was added.

The issue is that our requests, when we run xharness locally, will return
localpaths of the form: '/20250221_181807/index.html' that is because
the request is for 'http://localhost:51234/20250221_181807/index.html'.

In webapps, the path of the request ALWAYS starts with '/' even
when the request is for 'http://localhost:51234' (which returns '/').

In an attemp to fix xharness AND keep CodeQL happy I have made the
following changes:

1. Ensure that the full path is one of the allowed ones.
2. If the path is not one of the allowed ones, check if it is and
   allowed file.
2. Do not try to check for '/' or '\' but verify that the path instead
   get the path directory and ensure that is the one for the logs and no
   other.

This should fix xharness and keep CodeQL happy.
Copy link
Contributor

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build #089b301] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build #089b301] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [PR Build #089b301] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🔥 Failed to compare API and create generator diff 🔥

Error: 'make' failed for the hash 12d38e2.

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build #089b301] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build #534457d] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: 534457d5f0860ce44d4db4aed109ccc9f2c74174 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build #089b301] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

All tests on macOS arm64 - Mac Sequoia (15) passed.

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build #089b301] Tests on macOS M1 - Mac Ventura (13) passed 💻

All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build #089b301] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 112 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 8 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 089b301603ec0f2f145b58ff1855a4371c1b32ff [PR build]

Copy link
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

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

Ops... my bad, I thought I'd tested this.

Comment on lines +18 to +40
"/",
"/index.html",
"/set-option",
"/select",
"/deselect",
"/stop",
"/run",
"/build",
"/reload-devices",
"/reload-simulators",
"/quit",
"/favicon.ico",
"/index.html",
};

static IReadOnlySet<string> AllowedFiles = new HashSet<string> (StringComparer.Ordinal) {
"index.html",
"xharness.css",
"xharness.js",
};



Copy link
Member

Choose a reason for hiding this comment

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

Indentation is off here, and there are a few extra blank lines.

@mandel-macaque mandel-macaque merged commit 30cce24 into main Feb 24, 2025
39 of 42 checks passed
@mandel-macaque mandel-macaque deleted the dev/mandel/fix-xharness branch February 24, 2025 13:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants