Skip to content

Conversation

@zohrehKazemianpour
Copy link

You must title your PR like this:

Region | Cohort | FirstName LastName | Sprint | Assignment Title

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

I finished the middleware exercise. I made two versions to show the difference:

  • app.js: I wrote my own parser using req.on('data') and req.on('end'). This helps me understand how Node.js actually receives data chunks.

  • app-builtin.js: I swapped my manual code for express.json().

I spent some time debugging the curl commands. I found out that:

  1. My manual parser was accepted anything.

  2. express.json() is more strict. It only works if I add the -H "Content-Type: application/json" header to the curl command.
    Logic included

I also added code to check if the user is authenticated with X-Username and made sure the response handles plural words correctly.

@zohrehKazemianpour zohrehKazemianpour added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 8, 2026
@OracPrime OracPrime added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Feb 10, 2026
Copy link

@OracPrime OracPrime left a comment

Choose a reason for hiding this comment

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

One comment to understand.
One bug to at least understand. It's subtle enough you don't need to fix it unless you want a stretch task.
Otherwise solid code

// Header Middleware
const checkUsername = (req, res, next) => {
const usernameHeader = req.get("X-Username");
req.username = usernameHeader || null;

Choose a reason for hiding this comment

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

Are you deliberately using || instead of ?? here? Are you clear what the difference is? If yes to both, then no problem.

let rawBody = "";

req.on("data", (chunk) => {
rawBody += chunk;

Choose a reason for hiding this comment

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

There is a subtle bug here. chunk is bytes. When you do rawBody+=chunk it does a toString. However a single character in UTF8 can be up to 4 bytes. If your chunk ends in the middle of those 4 then the string conversion goes wrong. Better is to concatenate the byte chunks and convert once.

@OracPrime OracPrime added Reviewed Volunteer to add when completing a review with trainee action still to take. Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants