Skip to content
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

Remove default favicon #17925

Closed
vpavic opened this issue Aug 20, 2019 · 7 comments
Closed

Remove default favicon #17925

vpavic opened this issue Aug 20, 2019 · 7 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@vpavic
Copy link
Contributor

vpavic commented Aug 20, 2019

The default favicon served by Spring Boot could be classified as information leakage, in a similar manner like Server HTTP header (see #4730) and exception error attribute (see #7872) were.

I'd consider removing the default favicon as applications that don't provide custom favicon will inevitably leak info about being powered by Spring Boot.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 20, 2019
@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Aug 21, 2019
@wilkinsona
Copy link
Member

This is quite tempting. While we have a configuration property (spring.mvc.favicon.enabled), it's enabled by default. The docs also do not seem to mention that a default favicon will be served so some people may not be aware of the out-of-the-box behaviour.

If an application developer cares about the favicon they will provide their own. If they don't care about it I doubt there's much difference to them between serving a default icon and serving nothing.

@wilkinsona
Copy link
Member

wilkinsona commented Aug 21, 2019

Another benefit of removing the default favicon is that we could then also remove the spring.mvc.favicon.enabled property. It's a benefit as the property is confusing. Setting it to false does not, as the property's name might suggest, disable serving of a favicon.ico completely. It only disables serving a favicon.ico from the root of the classpath. A favicon.ico that's placed in one of the static resource locations will still be served.

@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Aug 21, 2019
@philwebb philwebb added this to the 2.2.x milestone Aug 21, 2019
@wilkinsona
Copy link
Member

wilkinsona commented Aug 21, 2019

We've decided to remove the default favicon.ico file, the resource handler configuration, and the property. Users who were placing a custom favicon.ico in the root of the classpath should move it to a static resource location or configure their own resource mapping.

@wilkinsona wilkinsona changed the title Consider removing default favicon Remove default favicon Aug 21, 2019
@wilkinsona wilkinsona self-assigned this Aug 22, 2019
@wilkinsona wilkinsona modified the milestones: 2.2.x, 2.2.0.M6 Aug 22, 2019
pull bot pushed a commit to scope-demo/spring-boot that referenced this issue Aug 22, 2019
@cardil

This comment has been minimized.

@patkovskyi
Copy link

patkovskyi commented Nov 1, 2019

What's the correct way to:

  1. Return 404 on /favicon.ico requests
  2. Return 200 with empty body on /favicon.ico requests

in Spring Boot 2.2.0+?

Note: I have spring.resources.add-mappings=false

@wilkinsona
Copy link
Member

wilkinsona commented Nov 1, 2019

@patkovskyi You should get a 404 by default if there are no mappings configured for /favicon.ico. To return a 200 with an empty body, you'd have to set up a mapping somehow, for example with a @ResponseBody-annotated @GetMapping that returns null.

If you have any further questions, please follow up on Stack Overflow or Gitter. As mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

@wilkinsona
Copy link
Member

@hanryusan That’s unrelated to this issue as, with or without a default favicon, the location will change if you configure a context path. In that case you should link to the favicon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants