search

openzipkin / zipkin-aws

Reporters and collectors for use in Amazon's cloud
Apache License 2.0
69 stars 34 forks source link

Different behavior during parsing of incorrect timestamp field in AWS TraceId #202

Closed GiuMontesano closed 11 months ago

GiuMontesano commented 1 year ago

Environment setup:

Describe the Bug

I found a different behavior when an incorrect timestamp field of AWS X-Amzn-Trace-Id header is parsed.

Behavior 1

Considering this incorrect TraceId: Root=1-z1373bbc-f1f4b48ef7ff3eebd62b5e01 (please note the leading z in the timestamp part of the TraceId, that is not a valid hexadecimal character). In this case the application does not throw any Exception, but it generates a new TraceId from scratch.

Behavior 2

Considering this second incorrect TraceId: Root=1-1z373bbc-f1f4b48ef7ff3eebd62b5e01 (please note the z in the second position of the timestamp part of the TraceId, that is not a valid hexadecimal character). In this case the application throws an java.lang.IllegalStateException: Missing: traceId.

Expected Behaviour

I expect the result of the first behavior in either cases (so, a new generated TraceId).

Steps to Reproduce

Behavior 1

  1. In order to test this behaviour you need to pass a request with following header: X-Amzn-Trace-Id: Root=1-z1373bbc-f1f4b48ef7ff3eebd62b5e01.
  2. In this case you can notice that a new TraceId is generated.

Behavior 2

  1. In order to test this behaviour you need to pass a request with following header: X-Amzn-Trace-Id: Root=1-1z373bbc-f1f4b48ef7ff3eebd62b5e01.
  2. In this case you can notice that a java.lang.IllegalStateException: Missing: traceId is thrown.

Notes

I think that this different behaviors is originated from this section of code.

Behavior 1

The first character is not hexadecimal, so the traceIdHigh (line 121) variable does not contain any value and the program goes direct to last else (line 126) of the section that make a break from the outer loop. When the program reaches the line 176 it goes to the positive branch of the if, and generates a new TraceId.

Behavior 2

The first character is hexadecimal, so the traceIdHigh is initialised from this character. However the second one is not hexadecimal, so the program goes in the same else of behavior 1, that make a break from the outer loop. When the program reaches the line 176 it goes into the negative branch of the if, in the line 185. This line of code generates an java.lang.IllegalStateException caused by an empty traceId variable passed to the builder.

codefromthecrypt commented 11 months ago

thanks for the fixes in #203