fix: resolve issue with processing content-length incorrectly

[rogaldh]

Description

This PR fixes issue with content-length header.

Type of change

• Bug fix
• New feature
• Protocol integration
• Documentation update
• Other (please describe):

Screenshots

None.

Testing

pnpm t

Related Issues

Relates to: #451

Checklist

• My code follows the project's style guidelines
• I have added tests that prove my fix/feature works
• All tests pass locally and in CI
• I have updated documentation as needed
• CI/CD checks pass
• I have included screenshots for protocol screens (if applicable)
• For security-related features, I have included links to related information

---

Important

Fixes handling of Content-Length header in GET function, ensuring correct behavior when present or absent, with tests added.

• Behavior:
  • Fixes handling of Content-Length header in GET function in route.ts.
  • Preserves Content-Length if present; omits if absent.
  • Returns 400 error if required headers are missing.
• Tests:
  • Adds tests in endpoint.test.ts for responses with and without Content-Length.
  • Verifies correct status and headers in both scenarios.
• Misc:
  • Renames test suite from metadata/[network] endpoint to Metadata Proxy Route.

^{This description was created by for 6252226. It will automatically update as commits are pushed.}

[vercel]

@rogaldh is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 6252226 in 2 minutes and 37 seconds

More details

• Looked at 133 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. app/api/metadata/proxy/route.ts:92

• Draft comment:
  Good use of temporary variable for content-length. However, note that if resourceHeaders.get returns an empty string, the check later (line 105) won’t detect it as missing. Consider more robust validation for header values.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment suggests checking for empty strings, but this isn't necessary because: 1) All required headers have non-empty fallback values, 2) Content-Length is optional, 3) The ?? operator ensures no null values. The validation check appears to be defensive programming but can never actually fail given the code structure.
  The comment author might be thinking about future maintainability if someone changes the fallback values. Also, they might be concerned about malformed headers from the original response.
  The current code structure with fallback values makes the suggested validation unnecessary. If the code structure changes in the future, that would be the time to add additional validation.
  The comment should be deleted because it suggests adding unnecessary validation - the current code structure with fallback values already ensures valid headers.

2. app/api/metadata/proxy/__tests__/endpoint.test.ts:143

• Draft comment:
  The test comment implies that Next.js should calculate and set Content-Length if missing, but the assertion expects it to be null. Consider clarifying whether this is the intended behavior or updating the comment to match the implementation.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  Looking at the full context, this test is specifically about handling responses without Content-Length headers. The test comment seems to be documenting a theoretical behavior of Next.js, while the test is asserting the actual current behavior. The second test shows that Content-Length is preserved when present, suggesting this behavior is intentional. The comment in the code appears to be documenting why we're checking for null, not indicating a bug.
  The comment could be right that there's unclear documentation here. Maybe the test comment should be reworded to be more precise about the expected behavior.
  While the test comment could be clearer, this is a documentation issue in test code, not a functional issue. The tests are clearly and intentionally testing the current behavior.
  The comment should be deleted. It's suggesting clarification of test documentation, which is not a critical issue requiring a code change.

3. app/api/metadata/proxy/route.ts:95

• Draft comment:
  Default Content-Type value now uses a semicolon instead of a comma. Ensure all clients and tests expect 'application/json; charset=utf-8' rather than 'application/json, charset=utf-8'.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

4. app/api/metadata/proxy/route.ts:99

• Draft comment:
  Good fix: conditionally adding the Content-Length header. Consider using an explicit null check (e.g. contentLength !== null) to clearly differentiate between a missing header and falsy values.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  Headers.get() returns string | null. An empty string content-length would be invalid for HTTP headers, so treating it as falsy is actually correct behavior. The current code's behavior matches the intent - we only want to set Content-Length if we got a valid value. The suggestion would make the code more explicit but doesn't fix any actual issues.
  The suggestion technically makes the code more explicit and type-safe. An empty string content-length would be caught earlier with the suggested approach.
  While more explicit, the current code correctly handles all real-world cases since an empty content-length header would be invalid anyway. The change doesn't provide meaningful value.
  Delete the comment as it suggests a style change that doesn't meaningfully improve the code's correctness or clarity.

5. app/api/metadata/proxy/__tests__/endpoint.test.ts:83

• Draft comment:
  Typo/clarity: The comment reads 'fail on encoded incorrectly input'. Consider rephrasing it to 'fail on incorrectly encoded input' to improve clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. app/api/metadata/proxy/route.ts:49

• Draft comment:
  There's a slight grammatical issue in the comment on line 49. Instead of 'despite of any other checks', consider rephrasing it to 'regardless of any other checks'. This will make the comment clearer.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_JxCurj2xpmxzufn3

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ngundotra]

Is it possible to also support metadata like this? I think a lot of NFT urls fail because IPFS content-type header for JSON documents is text/plain, not JSON (for example see https://raw.githubusercontent.com/solana-developers/opos-asset/main/assets/Climate/metadata.json)

http://localhost:3000/address/HwxZNMkZbZMeiu9Xnmc6Rg8jYgNsJB47jwabHGUebW4F?cluster=devnet

[ngundotra]

Another example that was failing in production w/o reason: https://explorer.solana.com/address/USDSwr9ApdHk5bvJKMjzff41FfuX8bSxdKcR81vTwcA