Skip to content

fix: linting issues#738

Merged
wolfy1339 merged 3 commits intomainfrom
fix-lint
Feb 13, 2025
Merged

fix: linting issues#738
wolfy1339 merged 3 commits intomainfrom
fix-lint

Conversation

@nickfloyd
Copy link
Copy Markdown
Contributor

@nickfloyd nickfloyd commented Feb 13, 2025

This is a blocker for the release to get previous merged work shipped.

@nickfloyd nickfloyd requested review from gr2m and wolfy1339 February 13, 2025 17:58
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 13, 2025

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@nickfloyd nickfloyd mentioned this pull request Feb 13, 2025
Merged
@wolfy1339
fix: update @octokit/endpoint and @octokit/request-error
e5e3d2c 
There were ReDos vulnerabilities in those packages
@wolfy1339 wolfy1339 enabled auto-merge (squash) February 13, 2025 18:05
@nickfloyd
Copy link
Copy Markdown
Contributor Author

nickfloyd commented Feb 13, 2025

Verified tests fail with previous versions of
"@octokit/endpoint": "^10.0.0",
"@octokit/request-error": "^6.0.1",

@wolfy1339
Copy link
Copy Markdown
Member

wolfy1339 commented Feb 13, 2025

I am seeing this locally as well. response.url is an empty string in the code, but is defined when i run fetch with the same url from a Node repl...

> await fetch("https://www.githubstatus.com/api/v2/status.json")
Response {
  status: 200,
  statusText: 'OK',
  headers: Headers {
    'content-type': 'application/json; charset=utf-8',
    'content-length': '215',
    connection: 'keep-alive',
    date: 'Thu, 13 Feb 2025 18:09:12 GMT',
    'x-download-options': 'noopen',
    'x-permitted-cross-domain-policies': 'none',
    'referrer-policy': 'strict-origin-when-cross-origin',
    'x-statuspage-version': '7edc5703ef05afb2a5eda55b3be76176673e3049',
    'strict-transport-security': 'max-age=259200',
    'x-statuspage-skip-logging': 'true',
    'access-control-allow-origin': '*',
    'cache-control': 'max-age=3, public',
    'x-pollinator-metadata-service': 'status-page-web-pages',
    'x-runtime': '0.049426',
    'accept-ranges': 'bytes',
    'x-content-type-options': 'nosniff',
    'x-xss-protection': '1; mode=block',
    'atl-traceid': 'fa5d78f462c64bad8bf75d0a81c6ac78',
    'atl-request-id': 'fa5d78f4-62c6-4bad-8bf7-5d0a81c6ac78',
    'report-to': '{"endpoints": [{"url": "https://dz8aopenkvv6s.cloudfront.net"}], "group": "endpoint-1", "include_subdomains": true, "max_age": 600}',
    nel: '{"failure_fraction": 0.001, "include_subdomains": true, "max_age": 600, "report_to": "endpoint-1"}',
    server: 'AtlassianEdge',
    etag: 'W/"4f979914dfba35b846c49883ff0e1feb"',
    vary: 'Accept,Accept-Encoding',
    'x-cache': 'Hit from cloudfront',
    via: '1.1 fbdc01f132101cb05310363b09502a86.cloudfront.net (CloudFront)',
    'x-amz-cf-pop': 'YUL62-P1',
    'x-amz-cf-id': 'H9TP5_pCul3H06Q7cpU1U_ajK2G7DuCUKfl5h29ID2nYAqC_czanAA==',
    age: '2'
  },
  body: ReadableStream { locked: false, state: 'readable', supportsBYOB: true },
  bodyUsed: false,
  ok: true,
  redirected: false,
  type: 'basic',
  url: 'https://www.github
  status.com/api/v2/status.json'
}

@nickfloyd
Copy link
Copy Markdown
Contributor Author

nickfloyd commented Feb 13, 2025 •
edited
Loading

I verified that the test is getting data back:

{
  "page": {
    "id": "kctbh9vrtdwd",
    "name": "GitHub",
    "time_zone": "Etc/UTC",
    "updated_at": "2025-02-13T15:05:54.233Z",
    "url": "https://www.githubstatus.com",
  },
  "status": {
    "description": "All Systems Operational",
    "indicator": "none",
  },
}

but response.url is always empty string. But the body has the host: expect(response.data.page.url).toEqual("https://www.githubstatus.com");

Feels like a redirect issue possibly, but the response.status value is 200 technically correct, if it was a redirect I would've expected a 3xx

@wolfy1339
Copy link
Copy Markdown
Member

wolfy1339 commented Feb 13, 2025

The last known good commit (even with updated versions of @octokit/request-error and @octokit/endpoint), was a0e96b3

The changes in 34ff07e seem to have broken the tests somehow

@nickfloyd
Copy link
Copy Markdown
Contributor Author

nickfloyd commented Feb 13, 2025 •
edited
Loading

Yeah it's something about the fetch-wrapper change that it does not like:
old (works):

  • responseHeaders.link.match(/<([^>]+)>;\s*rel="deprecation"/);
    new (breaks the test):
  • responseHeaders.link.match(/<([^<>]+)>; *rel="deprecation"/);

Looks like the \s match is the problem (or lack of in the new source)

@nickfloyd
Copy link
Copy Markdown
Contributor Author

nickfloyd commented Feb 13, 2025

I think I found it... It looks like the ReDoS tests are polluting the other request tests. I'm working on a fix now.

@wolfy1339 wolfy1339 merged commit 6bb29ba into main Feb 13, 2025
@wolfy1339 wolfy1339 deleted the fix-lint branch February 13, 2025 18:55
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 13, 2025

🎉 This PR is included in version 9.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wolfy1339 wolfy1339 wolfy1339 approved these changes

@gr2m gr2m Awaiting requested review from gr2m

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nickfloyd @wolfy1339