Skip to content

Add meaningful toString/getIpAddress methods to org.springframework.security.web.util.matcher.IpAddressMatcher #16693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
transentia opened this issue Mar 3, 2025 · 8 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@transentia
Copy link

Expected Behavior

It should be possible to determine the address that IpAddressMatcher was instantiated with. Via a getter or via toString.

Current Behavior

The best/only thing that one can get is:

org.springframework.security.web.util.matcher.IpAddressMatcher@4c18516

The class is final and cannot be overridden to supply these methods.
I guess this means that debugging would be a pain as well...

Looking at the code, it appears that the ipAddress parameter is not even retained after construction.

Context

I want to be able to read a list of matchers from an external config and then log the list once it has been created.

At the moment, I have to have use peek like this:

        // construct a whitelist for SecurityConfiguration and LumberjackUIAPI
        ipAddressMatcherWhitelist = CONF.values().stream()
                .filter(u -> u.getPort() != -1)  // ignore any entry that doesn't have a port
                .map(URI::getHost)
                .distinct()
                .map(StreamUtils.throwingFunctionWrapper(hostname -> InetAddress.getByName(hostname).getHostAddress()))
                // can't get value from IpAddressMatcher once constructed, so do logging here
                .peek(i -> log.info("Whitelisting server IP: {}", i))
                .map(IpAddressMatcher::new)
                .toList();

More cumbersome than I would like.

Less useful/informative than simply putting this after list creation:

log.info("Server whitelist: {}", ipAddressMatcherWhitelist);
@transentia transentia added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Mar 3, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Mar 6, 2025

I think a toString() implementation that shows the IP address and its mask, for example 10.0.0.0/8, makes sense. Are you able to contribute a PR?

@jzheaux jzheaux added status: ideal-for-contribution An issue that we actively are looking for someone to help us with and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 6, 2025
@wtigerhyunsu
Copy link
Contributor

I'd like to contribute to this issue. Would that be possible?
if possible, please assign it to me !!

@wtigerhyunsu
Copy link
Contributor

@transentia
Are you asking whether, when creating an object using new IpAddressMatcher, you want to be able to retrieve requiredAddress by calling IpAddressMatcher.getIpAddress() after registering it in IpAddressMatcher?

Do you just need a function inside IpAddressMatcher that returns the public requiredAddress?

@transentia
Copy link
Author

@wtigerhyunsu

At the time I opened this, I was just focussed on the toString.

I can definitely see the value of a getIpAddress method. also a getMask method, I guess.

And thank you for taking this on.

@wtigerhyunsu
Copy link
Contributor

@jzheaux
Since this is my first comment, I didn't realize that I needed to mention someone to notify them. So I'm commenting again.

Would it be okay if I contributed to this issue?

christopher-thumberger-whizus added a commit to christopher-thumberger-whizus/spring-security that referenced this issue Mar 20, 2025
Add meaningful toString() to IpAddressMatcher.java

Closes spring-projectsgh-16693

Signed-off-by: Christopher Thumberger <[email protected]>
@christopher-thumberger-whizus

@wtigerhyunsu jzheaux seems to rarely assigne issues. If it's something simple it's better to just do the issue and submit a PR than asking for assignment

@sjohnr
Copy link
Member

sjohnr commented Mar 20, 2025

@christopher-thumberger-whizus it looks like @wtigerhyunsu was interested in contributing but you opened your own PR. It also looks like your PR doesn't quite match what was mentioned in this comment. For those reasons, I will assign this issue to @wtigerhyunsu so they can try their hand at opening a PR.

@sjohnr sjohnr self-assigned this Mar 20, 2025
@sjohnr sjohnr added the in: web An issue in web modules (web, webmvc) label Mar 20, 2025
@sjohnr sjohnr assigned wtigerhyunsu and unassigned sjohnr Mar 20, 2025
wtigerhyunsu pushed a commit to wtigerhyunsu/spring-security that referenced this issue Mar 21, 2025
wtigerhyunsu pushed a commit to wtigerhyunsu/spring-security that referenced this issue Mar 21, 2025
@jzheaux jzheaux added status: duplicate A duplicate of another issue and removed status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Mar 21, 2025
@jzheaux
Copy link
Contributor

jzheaux commented Mar 21, 2025

Closing in favor of #16795

@jzheaux jzheaux closed this as completed Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment