Skip to content

fix: ngrok proxy endpoint URL handling#21

Merged
odanado merged 4 commits into
line:mainfrom
sumihiro:sumihiro-patch-1
Nov 5, 2025
Merged

fix: ngrok proxy endpoint URL handling#21
odanado merged 4 commits into
line:mainfrom
sumihiro:sumihiro-patch-1

Conversation

@sumihiro

Copy link
Copy Markdown
Contributor

Fix ngrok proxy endpoint URL handling

  • ensure ngrok forwarding always uses an explicit port by defaulting to 80/443 based on the incoming scheme
  • preserve the original endpoint pathname and query string when constructing the public tunnel URL so LIFF keeps non-root routes

Testing

  • liff-cli serve --proxy-type ngrok --url http://localhost/liff/some/path

Set default port for target URL in ngrok proxy
- ensure targetUrl.port falls back to 80 or 443 based on scheme so listener.forward always receives a valid host:port pair
- prevents ngrok from labeling the tunnel as app://... and avoids ERR_NGROK_3004 when the user omits the port in --url
Modify URL handling to include pathname and search parameters.
@sumihiro sumihiro requested a review from a team as a code owner October 20, 2025 01:44
@CLAassistant

CLAassistant commented Oct 20, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@nana4gonta nana4gonta added the status: triage Under Review label Oct 22, 2025

@odanado odanado left a comment

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.

@sumihiro Thank you for your contribution!
I’ve left one comment.

return new URL(url);
const urlWithPath = new URL(url);
urlWithPath.pathname = targetUrl.pathname;
urlWithPath.search = targetUrl.search;

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.

It's uncommon for LIFF app endpoint URLs to include a hash, but could you also include the hash in urlWithPath, just like pathname and search, to ensure that NgrokProxy correctly forwards the entire URL?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added urlWithPath.hash.

@odanado

odanado commented Nov 4, 2025

Copy link
Copy Markdown
Member

@sumihiro Could you check the CI error?

prettier --check .

Checking formatting...
[warn] src/serve/proxy/ngrok-proxy.ts
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

@sumihiro

sumihiro commented Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

@odanado I fixed it.

@odanado odanado left a comment

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.

Thank you!

@odanado odanado added the type: bug Bug Report label Nov 5, 2025
@odanado

odanado commented Nov 5, 2025

Copy link
Copy Markdown
Member

Thank you for your contribution!
Please wait a little longer for the new version to be released.

@odanado odanado merged commit 6fb2875 into line:main Nov 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: triage Under Review type: bug Bug Report

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants