Skip to content

context.request.url does not properly handle http2 #2147

@trentm

Description

@trentm

{transaction,span,error}.context.request.url is set in the agent by getContextFromRequest in lib/parsers.js: https://github.com/elastic/apm-agent-nodejs/blob/v3.17.0/lib/parsers.js#L18-L24

This uses the "original-url" package, to extract a URL object (in the form specified by the apm-server JSON specs -- https://github.com/elastic/apm-server/tree/master/docs/spec/v2/) from a node.js http server request object ((http.IncomingMessage)[https://nodejs.org/api/http.html#http_class_http_incomingmessage]).

With an http2 server the wrapped "stream" event does not have a direct IncomingMessage object. Currently the http2 instrumentation is setting this artificial req object on the transaction, which is later passed to original-url to create the URL fields. The result is something like { raw: '/', protocol: 'http:', pathname: '/' } (see this test code) instead of the more typical:

{
                        "raw": "/",
                        "protocol": "https:",
                        "hostname": "localhost",
                        "port": "$port",
                        "pathname": "/",
                        "full": "https://localhost:$port/"
                    },

from an https request.

Is that protocol: 'http:' wrong here? Shouldn't it be 'https:'?

Here is a smaller sample script that does the equivalent calls for dev testing:

const fs = require('fs')
const http2 = require('http2')
const originalUrl = require('original-url')

const server = http2.createSecureServer({
  allowHTTP1: true,
  key: fs.readFileSync('localhost-privkey.pem'),
  cert: fs.readFileSync('localhost-cert.pem')
})

server.on('stream', (stream, headers) => {
  stream.respond({
    'content-type': 'text/html; charset=utf-8',
    ':status': 200
  })
  const req = {
    headers,
    socket: stream.session.socket,
    method: headers[':method'],
    url: headers[':path'],
    httpVersion: '2.0'
  }
  const url = originalUrl(req)
  console.warn('XXX url: ', url)
  console.warn('XXX from req: ', req)
  stream.end(`Original URL: ${url.full}\n`)
})

server.listen(1337)

// 1. Create a local self-signed cert:
//      openssl req -x509 -newkey rsa:2048 -nodes -sha256 -subj '/CN=localhost' -keyout localhost-privkey.pem -out localhost-cert.pem
// 2. Run the server:
//      node playorigurl.js
// 3. Make an HTTP/2 request (use '-vv' curl option to see ALPN negotiation)
//      curl https://localhost:8443 -ki
//
// Also worth testing is an HTTP/1.1 request:
//      curl https://localhost:8443 -i --http1.1

Perhaps original-url could be updated (or the logic pulled into the agent) to support https://nodejs.org/api/http2.html classes like Http2Session and Http2Stream or the different HTTP/2 headers and/or the compatibility API classes http2.Http2ServerRequest (which would require instrumenting the "request" event).

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent-nodejsMake available for APM Agents project planning.bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions