Skip to content

Conversation

@kunwp1
Copy link
Contributor

@kunwp1 kunwp1 commented Oct 15, 2024

This PR addresses a port handling issue in URL generation to prevent invalid redirection. It introduces logic to check if the port is -1 (not specified), 80 (default HTTP port), or 443 (default HTTPS port). When the port matches any of these conditions, it is omitted from the URL.

Currently, even if the port number is not specified, it still appears in the generated URL, leading to incorrect redirection. This fix ensures that URLs are properly formatted to avoid such issues.

[The screenshot of the current issue]
Screenshot 2024-10-15 at 11 48 55 AM

@kunwp1 kunwp1 requested a review from aglinxinyuan October 15, 2024 18:51
@kunwp1 kunwp1 self-assigned this Oct 15, 2024
Copy link
Contributor

@aglinxinyuan aglinxinyuan left a comment

Choose a reason for hiding this comment

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

LGTM!

@kunwp1 kunwp1 merged commit fe0fdf0 into master Oct 22, 2024
@kunwp1 kunwp1 deleted the chris-email-notification-port-number branch October 22, 2024 08:50
PurelyBlank pushed a commit that referenced this pull request Dec 4, 2024
This PR addresses a port handling issue in URL generation to prevent
invalid redirection. It introduces logic to check if the port is `-1`
(not specified), `80` (default HTTP port), or `443` (default HTTPS
port). When the port matches any of these conditions, it is omitted from
the URL.

Currently, even if the port number is not specified, it still appears in
the generated URL, leading to incorrect redirection. This fix ensures
that URLs are properly formatted to avoid such issues.

[The screenshot of the current issue]
<img width="740" alt="Screenshot 2024-10-15 at 11 48 55 AM"
src="https://github.com/user-attachments/assets/2da0d8ad-a58f-48bb-8c22-6f128d36021f">

Co-authored-by: Kunwoo Park <kunwoopark@Kunwoos-MacBook-Pro.local>
Co-authored-by: Xinyuan Lin <xinyual3@uci.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants