Skip to content

Commit

Permalink
XWIKI-22694: The output of Document#getExternalURL() in the PDF cover…
Browse files Browse the repository at this point in the history 
… doesn't match the URL used to access XWiki

* Set the Forwarded HTTP header when opening the print preview page in the headless Chrome instance in order for the external URL to be computed properly.
  • Loading branch information
@mflorea
mflorea committed Dec 10, 2024
1 parent 109d265 commit b1e5e4f
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 9 deletions.
41 changes: 40 additions & 1 deletion ...-export-pdf-api/src/main/java/org/xwiki/export/pdf/browser/AbstractBrowserPDFPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Enumeration;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;

Expand All @@ -54,6 +56,10 @@
*/
public abstract class AbstractBrowserPDFPrinter implements PDFPrinter<URL>
{
private static final String HTTP_HEADER_FORWARDED = "Forwarded";

private static final String HTTP_HEADER_FORWARDED_FOR = "X-Forwarded-For";

@Inject
protected Logger logger;

Expand All @@ -75,6 +81,7 @@ public InputStream print(URL printPreviewURL) throws IOException
CookieFilterContext cookieFilterContext = findCookieFilterContext(printPreviewURL, browserTab);
Cookie[] cookies = getCookies(cookieFilterContext);
try {
browserTab.setExtraHTTPHeaders(getExtraHTTPHeaders(cookieFilterContext));
if (!browserTab.navigate(cookieFilterContext.getTargetURL(), cookies, true,
this.configuration.getPageReadyTimeout())) {
throw new IOException("Failed to load the print preview URL: " + cookieFilterContext.getTargetURL());
Expand Down Expand Up @@ -180,7 +187,7 @@ private Optional<CookieFilterContext> getCookieFilterContext(URL targetURL, bool
BrowserTab browserTab)
{
Optional<String> browserIPAddress = isFilterRequired ? getBrowserIPAddress(targetURL, browserTab)
: Optional.of(StringUtils.defaultString(getRequest().getHeader("X-Forwarded-For")));
: Optional.of(StringUtils.defaultString(getRequest().getHeader(HTTP_HEADER_FORWARDED_FOR)));
return browserIPAddress.map(ip -> new CookieFilterContext()
{
@Override
Expand Down Expand Up @@ -215,6 +222,38 @@ private Optional<String> getBrowserIPAddress(URL targetURL, BrowserTab browserTa
return Optional.empty();
}

private Map<String, List<String>> getExtraHTTPHeaders(CookieFilterContext cookieFilterContext)
{
HttpServletRequest request = getRequest();

List<String> forwarded = new LinkedList<>();
Enumeration<String> forwardedValues = request.getHeaders(HTTP_HEADER_FORWARDED);

This comment has been minimized.

Sign in to view

Copy link
@tmortagne

tmortagne Dec 12, 2024 •
edited
Loading

Member

Just copy/pasting the headers from the initial request are not going to work if the application server is accessed directly (not through an HTTP proxy).

It would probably be much safer to resolve the source base URL (HttpServletUtils#getSourceBaseURL) and pass it through the standard Forwarded HTTP header.
if (forwardedValues != null) {
forwardedValues.asIterator().forEachRemaining(forwarded::add);
}

String forwardedFor = request.getHeader(HTTP_HEADER_FORWARDED_FOR);
if (StringUtils.isBlank(forwardedFor)) {
forwardedFor = request.getRemoteAddr();
}

String host = request.getHeader("X-Forwarded-Host");
if (StringUtils.isBlank(host)) {
host = request.getHeader("Host");
}

String protocol = request.getHeader("X-Forwarded-Proto");
if (StringUtils.isBlank(protocol)) {
protocol = request.getScheme();
}

String lastForwarded = String.format("by=%s;for=%s;host=%s;proto=%s", cookieFilterContext.getBrowserIPAddress(),
forwardedFor, host, protocol);
forwarded.add(lastForwarded);

return Map.of(HTTP_HEADER_FORWARDED, forwarded);
}

@Override
public boolean isAvailable()
{
Expand Down
16 changes: 16 additions & 0 deletions .../xwiki-platform-export-pdf-api/src/main/java/org/xwiki/export/pdf/browser/BrowserTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.List;
import java.util.Map;

import javax.servlet.http.Cookie;

Expand All @@ -33,6 +35,20 @@
*/
public interface BrowserTab extends AutoCloseable
{
/**
* Sets the extra HTTP headers to use when requesting web pages in this browser tab.
*
* @param headers the extra HTTP headers to use when requesting web pages in this browser tab
* @since 15.10.16
* @since 16.4.6
* @since 16.10.2
* @since 17.0.0RC1
*/
default void setExtraHTTPHeaders(Map<String, List<String>> headers)
{
// Do nothing by default.
}

/**
* Navigates to the specified web page, optionally waiting for it to be ready (fully loaded).
*
Expand Down
29 changes: 29 additions & 0 deletions ...form-export-pdf-api/src/test/java/org/xwiki/export/pdf/browser/BrowserPDFPrinterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.net.URL;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -127,6 +128,10 @@ void print() throws Exception
Cookie[] cookies = new Cookie[] {mock(Cookie.class)};
when(this.request.getCookies()).thenReturn(cookies);

when(this.request.getRemoteAddr()).thenReturn("192.168.0.118");
when(this.request.getHeader("Host")).thenReturn("external:9293");
when(this.request.getScheme()).thenReturn("https");

when(this.browserManager.createIncognitoTab()).thenReturn(this.browserTab);
when(this.browserTab.navigate(new URL("http://xwiki-host:9293/xwiki/rest/client?media=json"))).thenReturn(true);
when(this.browserTab.getSource()).thenReturn("{\"ip\":\"172.12.0.3\"}");
Expand All @@ -152,6 +157,9 @@ public InputStream answer(InvocationOnMock invocation) throws Throwable
assertEquals("172.12.0.3", this.cookieFilterContextCaptor.getValue().getBrowserIPAddress());
assertEquals(browserPrintPreviewURL, this.cookieFilterContextCaptor.getValue().getTargetURL());

verify(this.browserTab).setExtraHTTPHeaders(
Map.of("Forwarded", List.of("by=172.12.0.3;for=192.168.0.118;host=external:9293;proto=https")));

// Only the configured XWiki URI should be used to get the browser IP address.
verify(this.browserTab, never()).navigate(new URL("http://external:9293/xwiki/rest/client?media=json"));
verify(this.browserTab).close();
Expand Down Expand Up @@ -213,6 +221,27 @@ void printWithXWikiSchemeAndPortSpecified() throws Exception
verify(this.browserTab).navigate(new URL("ftp://xwiki-host:8080/xwiki/rest/client?media=json"));
}

@Test
void printWithReverseProxy() throws Exception
{
URL printPreviewURL = new URL("http://external:9293/xwiki/bin/export/Some/Page?x=y#z");
URL browserPrintPreviewURL = new URL("http://xwiki-host:9293/xwiki/bin/export/Some/Page?x=y#z");

when(this.cookieFilter.isFilterRequired()).thenReturn(false);

when(this.request.getHeader("X-Forwarded-For")).thenReturn("192.168.0.117");
when(this.request.getHeader("Host")).thenReturn("external:9293");
when(this.request.getHeader("X-Forwarded-Proto")).thenReturn("ftp");

when(this.browserManager.createIncognitoTab()).thenReturn(this.browserTab);
when(this.browserTab.navigate(browserPrintPreviewURL, null, true, 30)).thenReturn(true);

this.printer.print(printPreviewURL);

verify(this.browserTab).setExtraHTTPHeaders(
Map.of("Forwarded", List.of("by=192.168.0.117;for=192.168.0.117;host=external:9293;proto=ftp")));
}

@Test
void isAvailable()
{
Expand Down
16 changes: 16 additions & 0 deletions ...form-export-pdf-default/src/main/java/org/xwiki/export/pdf/internal/chrome/ChromeTab.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.servlet.http.Cookie;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xwiki.export.pdf.PDFExportConfiguration;
Expand Down Expand Up @@ -256,4 +258,18 @@ private void setCookies(Cookie[] servletCookies, URL targetURL)
network.clearBrowserCookies();
network.setCookies(browserCookies);
}

@Override
public void setExtraHTTPHeaders(Map<String, List<String>> headers)
{
LOGGER.debug("Setting extra HTTP headers [{}].", headers);
Network network = this.tabDevToolsService.getNetwork();
network.enable();
// The documentation of setExtraHTTPHeaders is not clear about the type of value we can pass for a header (key).
// We tried passing a List<String> and a String[] but in both cases we got an exception: "Invalid header value,
// string expected". So we concatenate the values of a header with a comma.
Map<String, Object> extraHeaders = headers.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, entry -> StringUtils.join(entry.getValue(), ",")));
network.setExtraHTTPHeaders(extraHeaders);
}
}
22 changes: 14 additions & 8 deletions ...platform-export-pdf-test-docker/src/test/it/org/xwiki/export/pdf/test/ui/PDFExportIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
*/
package org.xwiki.export.pdf.test.ui;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.net.URL;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -54,12 +58,9 @@
import org.xwiki.test.docker.junit5.UITest;
import org.xwiki.test.ui.TestUtils;
import org.xwiki.test.ui.po.LiveTableElement;
import org.xwiki.test.ui.po.SuggestInputElement;
import org.xwiki.test.ui.po.ViewPage;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Tests for PDF export.
*
Expand Down Expand Up @@ -296,7 +297,8 @@ void exportWithCustomPDFTemplate(TestUtils setup, TestReference testReference, T
setup.createPage(testReference, "", "").createPage().createPageFromTemplate("My cool template", null, null,
"XWiki.PDFExport.TemplateProvider");
PDFTemplateEditPage templateEditPage = new PDFTemplateEditPage();
templateEditPage.setCover(templateEditPage.getCover().replace("<h1>", "<h1>Book: "));
templateEditPage.setCover(templateEditPage.getCover().replace("<h1>", "<h1>Book: ").replace("</p>",
"</p>\n<p>$escapetool.xml($tdoc.externalURL)</p>"));
templateEditPage
.setTableOfContents(templateEditPage.getTableOfContents().replace("core.pdf.tableOfContents", "Chapters"));
templateEditPage.setHeader(templateEditPage.getHeader().replaceFirst("<span ", "Chapter: <span "));
Expand All @@ -306,9 +308,11 @@ void exportWithCustomPDFTemplate(TestUtils setup, TestReference testReference, T
// Register the template in the PDF export administration section.
setup.loginAsSuperAdmin();
PDFExportAdministrationSectionPage adminSection = PDFExportAdministrationSectionPage.gotoPage();
adminSection.getTemplatesInput().sendKeys("my cool").waitForSuggestions()
.selectByVisibleText("My cool template");
adminSection.clickSave();
SuggestInputElement templatesInput = adminSection.getTemplatesInput();
if (!StringUtils.join(templatesInput.getValues(), ",").contains("My cool template")) {
templatesInput.sendKeys("my cool").waitForSuggestions().selectByVisibleText("My cool template");
adminSection.clickSave();
}

// We also have to give script rights to the template author because it was created based on the default one
// which contains scripts.
Expand All @@ -320,6 +324,7 @@ void exportWithCustomPDFTemplate(TestUtils setup, TestReference testReference, T
PDFExportOptionsModal exportOptions = PDFExportOptionsModal.open(new ViewPage());
exportOptions.getTemplateSelect().selectByVisibleText("My cool template");

String currentURL = setup.getDriver().getCurrentUrl().replaceAll("/WebHome.*", "/");
try (PDFDocument pdf = export(exportOptions, testConfiguration)) {
// Verify that the custom PDF template was used.

Expand All @@ -329,6 +334,7 @@ void exportWithCustomPDFTemplate(TestUtils setup, TestReference testReference, T
// Verify the custom cover page.
String coverPageText = pdf.getTextFromPage(0);
assertTrue(coverPageText.contains("Book: Parent"), "Unexpected cover page text: " + coverPageText);
assertTrue(coverPageText.contains(currentURL), "Unexpected cover page text: " + coverPageText);

// Verify the custom table of contents page.
String tocPageText = pdf.getTextFromPage(1);
Expand Down

0 comments on commit b1e5e4f

Please sign in to comment.