Skip to content

Allow redirect to external sites if CORS isn't enabled#1891

Open
amitaibu wants to merge 1 commit intomasterfrom
1889-redirect-no-cors
Open

Allow redirect to external sites if CORS isn't enabled#1891
amitaibu wants to merge 1 commit intomasterfrom
1889-redirect-no-cors

Conversation

@amitaibu
Copy link
Copy Markdown
Collaborator

fixes #1889

@amitaibu amitaibu requested a review from mpscholten January 11, 2024 08:06
Comment thread lib/IHP/static/helpers.js
// If the send method throws an exception, redirect to the form action URL.
// This can happen for example if redirecting to an external site, which doesn't have CORS enabled,
// so we can't use Ajax.
window.location.href = url;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't work for forms that are submitted via POST. I don't think this can be really fixed at all except by disabling the JS form submission in those forms

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I'll share my example code soon to show my use case

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Here's the example: amitaibu/ihp-cms-starter#35

If you navigate to http://localhost:8000 without the PR it will result with console error. With the PR you'll be redirected to the repo of one of the best Haskell frameworks! 😄

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@amitaibu
Copy link
Copy Markdown
Collaborator Author

@copilot can you resolve conflicts with master?

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 16, 2025

@amitaibu I've opened a new pull request, #2196, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix issue #1889 by allowing form submissions to redirect to external sites when CORS is not enabled. The implementation adds a try-catch block around the request.send() call to catch potential exceptions and fallback to a standard page redirect.

Key Changes:

  • Wraps request.send() in a try-catch block to handle potential errors
  • Falls back to window.location.href redirect if an exception is caught

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/IHP/static/helpers.js
// If the send method throws an exception, redirect to the form action URL.
// This can happen for example if redirecting to an external site, which doesn't have CORS enabled,
// so we can't use Ajax.
window.location.href = url;
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The variable url is only defined in the GET method branch (line 279), but this catch block can be reached in both GET and POST cases. For POST requests, url will be undefined, causing a potential error. Use formAction instead, which is available in both branches.

Suggested change
window.location.href = url;
window.location.href = formAction;

Copilot uses AI. Check for mistakes.
Comment thread lib/IHP/static/helpers.js
Comment on lines +300 to +306
try {
request.send(parameters.join('&'));
} catch (e) {
// If the send method throws an exception, redirect to the form action URL.
// This can happen for example if redirecting to an external site, which doesn't have CORS enabled,
// so we can't use Ajax.
window.location.href = url;
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

XMLHttpRequest.send() typically does not throw exceptions for CORS errors. CORS failures are asynchronous and trigger the 'onerror' event handler instead. The try-catch block may not catch CORS-related issues as intended. Consider adding a request.onerror handler or a timeout mechanism to detect when AJAX submission fails and fallback to a regular form submission.

Suggested change
try {
request.send(parameters.join('&'));
} catch (e) {
// If the send method throws an exception, redirect to the form action URL.
// This can happen for example if redirecting to an external site, which doesn't have CORS enabled,
// so we can't use Ajax.
window.location.href = url;
// Fallback handler to ensure redirect only happens once
var hasFallenBack = false;
function fallbackToRegularSubmission() {
if (!hasFallenBack) {
hasFallenBack = true;
window.location.href = url;
}
}
request.onerror = fallbackToRegularSubmission;
request.ontimeout = fallbackToRegularSubmission;
// Set a timeout (e.g., 10 seconds)
request.timeout = 10000;
try {
request.send(parameters.join('&'));
} catch (e) {
// If the send method throws an exception, redirect to the form action URL.
// This can happen for example if redirecting to an external site, which doesn't have CORS enabled,
// so we can't use Ajax.
fallbackToRegularSubmission();

Copilot uses AI. Check for mistakes.
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.

Can't redirect to remote server

4 participants