Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Add Parameter Check for Negative Page Number #1503

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

TomiFixit
Copy link
Contributor

@TomiFixit TomiFixit commented Mar 24, 2023

Closes #1492

QA

If the page number is invalid (<0), the parameter is ignored.

@vercel
Copy link

vercel bot commented Mar 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-commerce ✅ Ready (Inspect) Visit Preview Mar 24, 2023 at 8:12PM (UTC)
react-commerce-prod ✅ Ready (Inspect) Visit Preview Mar 24, 2023 at 8:12PM (UTC)

@github-actions
Copy link
Contributor

📦 Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 353.52 KB (🟡 +4 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Copy link
Contributor

@ardelato ardelato left a comment

Choose a reason for hiding this comment

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

CR 👍 but deploy_block ☁️ on minor suggestion.

Also is Add supposed to be the full title of this PR?

p:
typeof p === 'string' && parseInt(p) >= 0
? parseInt(p)
: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the code it seems that p can by of type any.

However, parseInt() will return NaN if it can't parse the integer https://developer.mozilla.org/en-US/docs/web/javascript/reference/global_objects/parseint

So we can extract the parsing to a line above and then simply check the negative value in this line. This will make the line less bloated and not have to run parseInt twice.

So something like


const pageParam = parseInt(p)

return {
   q: String(q || ''),

   p: pageParam >= 0 ? pageParam : undefined

Typescript Example

@TomiFixit TomiFixit changed the title Add Add Parameter Check for Negative Page Number Mar 24, 2023
@ardelato
Copy link
Contributor

un_deploy_block ⚡ spoke with @TomiFixit and p was not just any so we would still need to type check the value. Therefore, the current way works.

@jordycosta
Copy link
Member

jordycosta commented Mar 27, 2023

QA 🟢

Using a negative page number with the ?p param no longer results in a 500 Internal Server Error 👍


Testing Procedure:

  1. Navigate to the Vercel preview
  2. Scroll down, select a different page number
  3. Modify the URL parameter to use a negative number (e.g. ?p=-100)
  4. Ensure that the parameter is ignored (i.e. no server error occurs)

@davidrans davidrans merged commit 525370c into main Mar 27, 2023
@davidrans davidrans deleted the handle-bad-page-number-url branch March 27, 2023 16:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad page number in url results in 500
4 participants