Skip to content

Fix HttpServletRequest.getRemoteAddr for the HttpConnector mode - #311

Merged
lachlan-roberts merged 2 commits into
mainfrom
getRemoteAddress
Nov 19, 2024
Merged

Fix HttpServletRequest.getRemoteAddr for the HttpConnector mode#311
lachlan-roberts merged 2 commits into
mainfrom
getRemoteAddress

Conversation

@lachlan-roberts

Copy link
Copy Markdown
Collaborator

Add RemoteAddressTest to test getRemoteAddr, getLocalAddr, getServerPort, getRemotePort, getLocalPort and getServerName for jetty94, EE8 and EE10 over both RPC and HTTP modes.

Add fixes in JettyRequestAPIData implementations to fix this for the HTTP mode.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

@Override
public String getRemoteAddr() {
return finalUserIp;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Why not just use userIp directly. It doesn't mutate between these lines right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not possible because it is reassigned above and the variable has to be "effectively final", the compiler will give the error:

Variable 'userIp' is accessed from within inner class, needs to be final or effectively final

Comment on lines +336 to +359
@Override
public String getServerName() {
return "0.0.0.0";
}

@Override
public String getRemoteHost() {
return finalUserIp;
}

@Override
public int getRemotePort() {
return 0;
}

@Override
public String getLocalName() {
return "0.0.0.0";
}

@Override
public String getLocalAddr() {
return "0.0.0.0";
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Would prefer to have these as String constants since they are repeated (similar to `WARMUP_IP)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>
@lachlan-roberts
lachlan-roberts merged commit 7f872a6 into main Nov 19, 2024
@lachlan-roberts
lachlan-roberts deleted the getRemoteAddress branch November 19, 2024 00:36
ludoch pushed a commit that referenced this pull request Nov 22, 2024
PiperOrigin-RevId: 697805351
Change-Id: Iad4396c3346c88f4bba8aa93a2648815fe358c85
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants