Setting user header on CONNECT message stopped working [SPR-15822]

[spring-projects-issues]

Jeff opened SPR-15822 and commented

We followed instructions in Token-Based Authentication in Spring doc at http://docs.spring.io/spring/docs/5.0.0.M5/spring-framework-reference/html/websocket.html#websocket-stomp-authentication-token-based, in order to set the user header on the CONNECT Message. In Spring framework 4.3.9, it works well. After migrating to 5.0.0 RC2, we found it stopped working. After some investigation, we tend to believe it is a bug in 5.0.0 RC2 as well as RC3.

[5.0.0 RC2 behavior] - bad
In method handleMessageFromClient() in StompSubProtocolHandler class, Principal is retrieved from session. Of course, at this point, the Principal is null. And then, Spring attempts to put Principal to stompAuthentications. Since it is null, nothing would be put into stompAuthentications.

			Principal user = getUser(session);
			if (user != null) {
				headerAccessor.setUser(user);
			}
			...
			try {
				SimpAttributesContextHolder.setAttributesFromMessage(message);
				boolean sent = outputChannel.send(message);

				if (sent) {
					if (isConnect) {
						if (user != null && user != session.getPrincipal()) {
							this.stompAuthentications.put(session.getId(), user);
						}
					}

[4.3.9 behavior] - good
Spring tries to retrieve Principal from STOMP header, and then put it into stompAuthentications.

if (sent) {
         if (isConnect) {
                  Principal user = headerAccessor.getUser();
                  if (user != null && user != session.getPrincipal()) {
                           this.stompAuthentications.put(session.getId(), user);
                  }
         }

This commit (f813712#diff-7bc1370febf168db39f9b3a608f68fe8) caused this regression. FYI.

---

Affects: 5.0 RC2, 5.0 RC3

Issue Links:

• Introduce null-safety of Spring Framework API [SPR-15540] #20099 Introduce null-safety of Spring Framework API

Referenced from: commits 25e6a2d

[spring-projects-issues]

Juergen Hoeller commented

Good catch: We need to re-retrieve the user at that point, which got accidentally dropped during that nullability refactoring. Restored for 5.0 RC4 now.