[Veritaris]

Hi
I recently tried to use you lib, and after our automatic passwords rotation mentioned an issue while connecting to the Artemis
I found out that you lib doesn't always follows the STOMP spec (described below)

Summary

Fix STOMP 1.2 header serialization for CONNECT and CONNECTED frames.

The frame writer currently applies STOMP header escaping to every frame.
For example, a password containing colons:

xxx-test:password:xxx

is currently sent as:

xxx-test\cpassword\cxxx

Brokers following the STOMP specification (e.g. ArtemisMQ) interpret that as a different
password, causing authentication to fail.

Specification

The STOMP 1.2 specification explicitly excludes CONNECT and CONNECTED
frames from header escaping:

All frames except the CONNECT and CONNECTED frames will escape any
carriage returns, line feeds or colons found in the resulting UTF-8
encoded headers.

Source: STOMP 1.2 — Value Encoding (https://stomp.github.io/stomp-specification-1.2.html#Value_Encoding)

Accordingly, credentials and other headers in these frames must be written
literally. Header escaping remains applicable to STOMP, SEND, and all
other frame commands.

Changes

• Write CONNECT and CONNECTED header names and values without STOMP
  escaping.

• Preserve the existing escaping behavior for every other frame command.

• Validate unescaped headers before writing:

  • Header names must be non-empty and cannot contain :, CR, LF, or NUL.
  • Header values cannot contain CR, LF, or NUL.
  • Colons and backslashes remain valid in header values.

• Return ErrInvalidFrameFormat before writing any partial frame when an
  invalid unescaped header is encountered.

• Leave frame parsing behavior unchanged.

Tests

Added coverage verifying that:

• CONNECT credentials containing colons and backslashes are sent
  literally.

• CONNECTED headers are sent literally.

• STOMP and SEND headers continue to be escaped.

• Invalid unescaped header names and values are rejected.

• Validation failures do not write a partial frame.

Validation performed:

go test ./...
go test -race ./...
go vet ./...

The change was also verified against Apache ActiveMQ Artemis 2.44 using a
password containing =, /, :, -, and +.

[Zerpet]

From the specification document:

The CONNECT and CONNECTED frames do not escape the carriage return, line feed or colon octets in order to remain backward compatible with STOMP 1.0.

We claim support for STOMP 1.0, 1.1 and 1.2, so we should address this. I'll try to take a look this week.

[Veritaris]

Hi!
Yeah, this is what my PR already does - in the CONNECT / CONNECTED frame abovementioned octets are not escaped, all as the spec describes 😁

[Zerpet]

Looks like there are some conflicts, probably due to #154. Can you resolve them?

[Veritaris]

Oh, definitely, I'l review conflicts today after working hours

[Veritaris]

Resolved conflicts in the tests

[Zerpet]

Looking good. I just have a question on escaping the value of a header. See below.

[Zerpet]

I'm wondering why the character : was not included in the check for the value? 🤔

I re-read the spec and I understand that both header keys and values must be escaped. Therefore, unescaped headers must not contain the colon character IIUC.

[mkuratczyk]

Thanks for the contribution. Your commit was included in #158.