morgan icon indicating copy to clipboard operation
morgan copied to clipboard

fix issue in field missing :req token

Open JabSYsEmb opened this issue 2 years ago • 4 comments

 var header = req.headers[field.toLowerCase()]
                                 ^
TypeError: Cannot read properties of undefined (reading 'toLowerCase')

Node.js v18.7.0

Missing field of req token causes the app to crash in this PR I fixed this issues with the following solution:

  • In case user doesn't define a specific field of the response the code will log the whole header response
  • If user defines its field the code will log just the value responding to the field itself.

I believe the code become more generic and it behaves correctly with various response headers.

All tests pass:

> [email protected] test
> mocha --check-leaks --reporter spec --bail



  morgan()
    arguments
      ✓ should use default format
      format
        ✓ should accept format as format name
        ✓ should accept format as format string
        ✓ should accept format as function
        ✓ should reject format as bool
        back-compat
          ✓ should accept options object
          ✓ should accept format in options for back-compat
          ✓ should accept format function in options for back-compat
      stream
        ✓ should default to process.stdout
        ✓ should set stream to write logs to
    tokens
      :date
        ✓ should get current date in "web" format by default
        ✓ should get current date in "clf" format
        ✓ should get current date in "iso" format
        ✓ should get current date in "web" format
        ✓ should be blank for unknown format
      :http-version
        ✓ should be 1.0 or 1.1
      :req
        ✓ should get request properties
        ✓ should display all values of array headers
      :res
        ✓ should get response properties
        ✓ should display all values of array headers
      :remote-addr
        ✓ should get remote address
        ✓ should use req.ip if there
        ✓ should work on https server
        ✓ should work when connection: close
        ✓ should work when connection: keep-alive
        ✓ should work when req.ip is a getter
        ✓ should not fail if req.connection missing
      :remote-user
        ✓ should be empty if none present
        ✓ should support Basic authorization
        ✓ should be empty for empty Basic authorization user
      :response-time
        ✓ should be in milliseconds
        ✓ should have three digits by default
        ✓ should have five digits with argument "5"
        ✓ should have no digits with argument "0"
        ✓ should not include response write time (51ms)
        ✓ should be empty without hidden property
        ✓ should be empty before response
        ✓ should be empty if morgan invoked after response sent
      :status
        ✓ should get response status
        ✓ should not exist before response sent
        ✓ should not exist for aborted request
      :total-time
        ✓ should be in milliseconds
        ✓ should have three digits by default
        ✓ should have five digits with argument "5"
        ✓ should have no digits with argument "0"
        ✓ should include response write time (52ms)
        ✓ should be empty without hidden property
        ✓ should be empty before response
        ✓ should be empty if morgan invoked after response sent
      :url
        ✓ should get request URL
        ✓ should use req.originalUrl if exists
        ✓ should not exist for aborted request
    formats
      a function
        ✓ should log result of function
        ✓ should not log for undefined return
        ✓ should not log for null return
      a string
        ✓ should accept format as format string of tokens
        ✓ should accept text mixed with tokens
        ✓ should accept special characters
      combined
        ✓ should match expectations
      common
        ✓ should match expectations
      default
        ✓ should match expectations
      dev
        ✓ should not color 1xx
        ✓ should color 2xx green
        ✓ should color 3xx cyan
        ✓ should color 4xx yelow
        ✓ should color 5xx red
        with "immediate: true" option
          ✓ should not have color or response values
      short
        ✓ should match expectations
      tiny
        ✓ should match expectations
    with buffer option
      ✓ should flush log periodically (1003ms)
      ✓ should accept custom interval (206ms)
    with immediate option
      ✓ should not have value for :res
      ✓ should not have value for :response-time
      ✓ should not have value for :status
      ✓ should log before response
    with skip option
      ✓ should be able to skip based on request
      ✓ should be able to skip based on response

  morgan.compile(format)
    arguments
      format
        ✓ should be required
        ✓ should reject functions
        ✓ should reject numbers
        ✓ should compile a string into a function


  81 passing (1s)

JabSYsEmb avatar Aug 25 '22 20:08 JabSYsEmb

Hello, and thank you for your PR. The crash is expected, as the :req token requires a header name. The error mesaage is not great, however. I think we can improve the error message that the argument is missing.

dougwilson avatar Aug 25 '22 20:08 dougwilson

Thank you for your review. I did fix the issue by throwing an error says 'req is missing an argument'.

:req may take the following arguments : host, 'accept-encoding', 'user-agent','set-cookie', connection

JabSYsEmb avatar Aug 26 '22 07:08 JabSYsEmb

@dougwilson, any updates?

JabSYsEmb avatar Sep 17 '22 20:09 JabSYsEmb

@dougwilson, thank you for your suggestions. I did try to write a unit test but I wasn't able to figure it out. can you help me figuring it out?

JabSYsEmb avatar Sep 19 '22 08:09 JabSYsEmb

@dougwilson, I think I am not able to add tests if you point me to a resource to figure out how could I write a proper tests I'll be thankful.

JabSYsEmb avatar Sep 22 '22 10:09 JabSYsEmb