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.

Sign up for GitHub

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

Jump to bottom

Premature invocation of h:outputLink on Firefox #5414

Closed
liefke opened this issue Mar 6, 2024 · 4 comments
Closed

Premature invocation of h:outputLink on Firefox #5414

liefke opened this issue Mar 6, 2024 · 4 comments
Milestone
4.0.7

Comments

@liefke
Copy link

liefke commented Mar 6, 2024 •
edited

Describe the bug

When a browser and server with HTTP/2 Push support is used (like FireFox and WildFly), all URLs referenced by an <h:outputLink value="..."> are pushed to the client, which leads to premature requests for these resources.

To Reproduce

Steps to reproduce the behavior:

  1. Create a webapp with this index.xhtml:

     <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
 <f:view xmlns:f="http://java.sun.com/jsf/core" xmlns:h="http://java.sun.com/jsf/html">
 <html xmlns="http://www.w3.org/1999/xhtml">
     <h:head /> 
     <h:body>
         <h:outputLink value="/logout">Logout</h:outputLink><br/>
         <h:outputLink value="https://www.google.de/" target="_BLANK">Google</h:outputLink><br/>
         <h:outputLink value="/">Home</h:outputLink><br/>
     </h:body>
 </html>
 </f:view>

  2. Turn on the access log of your server (I used WildFly 31.0.1.Final for testing).

  3. Start the server and deploy the webapp

  4. Open your webapp in the current Firefox 123.0.1 using HTTPS (to trigger usage of HTTP/2)

  5. Check the access logs.

  6. You will find the following:

     "GET / HTTP/2.0" 200 252
 "GET /logout HTTP/1.1" 404 68
 "GET /https://www.google.de/ HTTP/1.1" 404 68
 "GET / HTTP/1.1" 200 252

As you can see, each outputLink is "prefetched" by Firefox, even the external URL.

In combination with some redirects in my application I even managed (unintentionally) to produce an endless loop of requests.

Expected behavior

I only see one request for "/" in the access log, nothing else.

Versions

  • Browser: FireFox 123.0.1
  • JSF: Jakarta Faces 4.0.5

Workarounds

Turn off HTTP/2 Push in your server, example for the standalone.xml in WildFly:

<https-listener ... http2-enable-push="false" />

Or create an ExternalContext wrapper in your application and overwrite encodeResourceURL:

public String encodeResourceURL(String url) {
    return ((HttpServletResponse) getResponse()).encodeURL(url);
}

Analysis

I would never expect a HTTP/2 Push for URLs of a h:commandLink. If necessary at all, JSF should only push resources of h:graphicImage and should not use ExternalContextImpl.encodeResourceUrl for that purpose, as the push is an unexpected side effect of a call to that method. In addition even h:graphicImage should check, if the URL is an external URL.

Nevertheless I would remove HTTP/2 Push support from JSF at all, see the quote here:

In practice, Server Push frequently results in wasted bandwidth because the server rarely knows which resources are already loaded by the client and transmits the same resource multiple times, resulting in slowdowns if the resources being pushed compete for bandwidth with resources that were requested.
@liefke
Copy link
Author

liefke commented Mar 6, 2024

By the way, I'd guess that the current Push implementation is even the root cause for #5391, at least on Firefox.

@liefke liefke changed the title Premature invocation of outputLink on FireFox Premature invocation of h:outputLink on Firefox Mar 6, 2024
@BalusC
Copy link
Contributor

BalusC commented Mar 24, 2024

This is part of the spec: https://jakarta.ee/specifications/faces/4.0/jakarta-faces-4.0#a457

If running on a container that supports Jakarta Servlet 4.0 or later, after any dynamic component manipulations have been completed, any resources that have been added to the UIViewRoot, such as scripts, images, or stylesheets, and any inline images, must be pushed to the client using the Jakarta Servlet Server Push API. All of the pushes must be started before any of the HTML of the response is rendered to the client.

So removing is not an option. However, you're right that it should not have pushed commandlinks let alone external resources. The implementation is indeed incorrect in this regard.

@BalusC BalusC mentioned this issue Mar 24, 2024
Merged
BalusC added a commit that referenced this issue Mar 30, 2024
@BalusC
Merge #5414, #5420 and #5427 from 4.0 into 4.1
ab98cc3
@BalusC BalusC closed this as completed in 93aea33 Mar 30, 2024
BalusC added a commit that referenced this issue Mar 30, 2024
@BalusC
Merge #5414, #5420 and #5427 from 4.1 into 5.0
65e7b8a
@BalusC BalusC added this to the 4.0.7 milestone Mar 30, 2024
@liefke
Copy link
Author

liefke commented Apr 11, 2024

Thanks for fixing this.

But the reference to the spec is only half the truth - as you could deal with the issue in the next release of the spec. I guess that you understand the argument of the wasted bandwidth and that HTTP/2 Push is a dead end.

Nevertheless I still think, that encodeResourceURL is the wrong place for initiating the push, as the method could be called for any resource, not only for resources that have been added to the UIViewRoot (as required by the spec).

@BalusC
Copy link
Contributor

BalusC commented Apr 11, 2024

Fair point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.0.7
Development

No branches or pull requests
2 participants
@BalusC @liefke