Skip to content

tool_ipfs: accept IPFS gateway URL without set port number#20957

Closed
bagder wants to merge 3 commits intomasterfrom
bagder/ipfs-gateway-port
Closed

tool_ipfs: accept IPFS gateway URL without set port number#20957
bagder wants to merge 3 commits intomasterfrom
bagder/ipfs-gateway-port

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 17, 2026

Follow-up to: 5612971

Test 1851 added to verify

Pointed out by Codex Security

Follow-up to: 5612971

Test 1851 added to verify

Pointed out by Codex Security
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 17, 2026

augment review

@bagder bagder marked this pull request as ready for review March 17, 2026 12:43
@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 17, 2026

🤖 Augment PR Summary

Summary: Makes curl’s IPFS gateway handling accept gateway URLs that omit an explicit port.

Changes:

  • Read the gateway port with CURLU_DEFAULT_PORT so http/https defaults are applied when no port is present.
  • Add test 1851 to verify --ipfs-gateway http://… (no port) rewrites and requests correctly.
  • Register the new test in tests/data/Makefile.am.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

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

Updates curl’s IPFS URL rewriting to accept gateway URLs that omit an explicit port number, and adds a regression test to validate the behavior.

Changes:

  • Adjust ipfs_url_rewrite() to fetch the gateway port using CURLU_DEFAULT_PORT when none is explicitly provided.
  • Add new test case test1851 to verify rewriting through a proxy when the gateway URL has no port specified.
  • Register the new test in tests/data/Makefile.am.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/data/test1851 New IPFS regression test exercising gateway URL rewriting without an explicit port
tests/data/Makefile.am Adds test1851 to the testcases list
src/tool_ipfs.c Uses CURLU_DEFAULT_PORT when extracting the gateway port

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

You can also share your feedback on Copilot code review. Take the survey.

Comment thread tests/data/test1851 Outdated
Comment thread tests/data/test1851
Comment thread src/tool_ipfs.c Outdated
@testclutch
Copy link
Copy Markdown

Analysis of PR #20957 at 8477679b:

Test 1851 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 18 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@bagder bagder closed this in f4c0590 Mar 17, 2026
@bagder bagder deleted the bagder/ipfs-gateway-port branch March 17, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants