Fix RootedDocServlet generating hyperlinks
Commit c8fac64291b8 ("Fix relative hyperlinks in Markdown") broke
gerritcodereview.com and other RootedDocServlet use cases due to a
misuse of the GitilesView for href generation.
Pass through the actual file path read from the repository to the
MarkdownToHtml for href link resolution. This avoids relying on weird
corner cases in the GitilesView -> Markdown mapping like README.md or
index.md. With all hrefs generated using / from the host root, the
view path isn't very relevant.
Add tests for the missing case of the GitilesView being ROOTED_DOC
type and not PATH type. The ROOTED_DOC does not have a repositoryName
property and this is what caused gerritcodereview.com to not render
correctly.
Change-Id: I63b47f1b2d25a5580c28134fa17517d540961c43
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ReadmeHelper.java b/gitiles-servlet/src/main/java/com/google/gitiles/ReadmeHelper.java
index 6bf0feb..10b2048 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ReadmeHelper.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ReadmeHelper.java
@@ -107,7 +107,7 @@
img = new ImageLoader(reader, view, rootTree, readmePath, imageLimit);
}
- return new MarkdownToHtml(view, cfg).setImageLoader(img).setReadme(true).toSoyHtml(root);
+ return new MarkdownToHtml(view, cfg, readmePath).setImageLoader(img).toSoyHtml(root);
} catch (LargeObjectException | IOException e) {
log.error(String.format("error rendering %s/%s", view.getRepositoryName(), readmePath), e);
return null;
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/doc/DocServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/doc/DocServlet.java
index 4b774a5..bdb28a4 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/doc/DocServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/doc/DocServlet.java
@@ -122,11 +122,13 @@
return;
}
+ String navPath = null;
RootNode nav = null;
if (navmd != null) {
+ navPath = navmd.path;
nav =
GitilesMarkdown.parseFile(
- parseTimeout, view, navmd.path, navmd.read(rw.getObjectReader(), inputLimit));
+ parseTimeout, view, navPath, navmd.read(rw.getObjectReader(), inputLimit));
if (nav == null) {
res.setStatus(SC_INTERNAL_SERVER_ERROR);
return;
@@ -140,7 +142,7 @@
}
res.setHeader(HttpHeaders.ETAG, curEtag);
- showDoc(req, res, view, cfg, img, nav, doc);
+ showDoc(req, res, view, cfg, img, navPath, nav, srcmd.path, doc);
}
}
@@ -176,7 +178,9 @@
GitilesView view,
Config cfg,
ImageLoader img,
+ String navPath,
RootNode nav,
+ String docPath,
RootNode doc)
throws IOException {
Map<String, Object> data = new HashMap<>();
@@ -187,8 +191,8 @@
data.put("logUrl", GitilesView.log().copyFrom(view).toUrl());
data.put("blameUrl", GitilesView.blame().copyFrom(view).toUrl());
}
- data.put("navbarHtml", new MarkdownToHtml(view, cfg).toSoyHtml(nav));
- data.put("bodyHtml", new MarkdownToHtml(view, cfg).setImageLoader(img).toSoyHtml(doc));
+ data.put("navbarHtml", new MarkdownToHtml(view, cfg, navPath).toSoyHtml(nav));
+ data.put("bodyHtml", new MarkdownToHtml(view, cfg, docPath).setImageLoader(img).toSoyHtml(doc));
String analyticsId = cfg.getString("google", null, "analyticsId");
if (!Strings.isNullOrEmpty(analyticsId)) {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
index 1b5df78..078ee1e 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
@@ -18,7 +18,6 @@
import static com.google.gitiles.doc.MarkdownUtil.getInnerText;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.gitiles.GitilesView;
import com.google.gitiles.ThreadSafePrettifyParser;
@@ -84,14 +83,26 @@
private final TocFormatter toc = new TocFormatter(html, 3);
private final GitilesView view;
private final Config cfg;
+ private final String filePath;
private ImageLoader imageLoader;
- private boolean readme;
private TableState table;
private boolean outputNamedAnchor = true;
- public MarkdownToHtml(GitilesView view, Config cfg) {
+ /**
+ * Initialize a Markdown to HTML converter.
+ *
+ * @param view view used to access this Markdown on the web. Some elements of
+ * the view may be used to generate hyperlinks to other files, e.g.
+ * repository name and revision.
+ * @param cfg
+ * @param filePath actual path of the Markdown file in the Git repository. This must
+ * always be a file, e.g. {@code doc/README.md}. The path is used to
+ * resolve relative links within the repository.
+ */
+ public MarkdownToHtml(GitilesView view, Config cfg, String filePath) {
this.view = view;
this.cfg = cfg;
+ this.filePath = filePath;
}
public MarkdownToHtml setImageLoader(ImageLoader img) {
@@ -99,11 +110,6 @@
return this;
}
- public MarkdownToHtml setReadme(boolean readme) {
- this.readme = readme;
- return this;
- }
-
/** Render the document AST to sanitized HTML. */
public SanitizedContent toSoyHtml(RootNode node) {
if (node == null) {
@@ -373,17 +379,7 @@
return toPath(target);
}
- String viewPath = Strings.nullToEmpty(view.getPathPart());
- String dir;
- if (readme) {
- dir = CharMatcher.is('/').trimTrailingFrom(viewPath);
- } else {
- // When readme is false this instance is rendering a file, whose name
- // appears as the last component of viewPath. Other links are relative
- // to the file's container directory, so trim the file.
- dir = trimLastComponent(viewPath);
- }
-
+ String dir = trimLastComponent(filePath);
while (!target.isEmpty()) {
if (target.startsWith("../") || target.equals("..")) {
if (dir.isEmpty()) {
@@ -408,7 +404,13 @@
}
private String toPath(String path) {
- return GitilesView.path().copyFrom(view).setPathPart(path).build().toUrl();
+ GitilesView.Builder b;
+ if (view.getType() == GitilesView.Type.ROOTED_DOC) {
+ b = GitilesView.rootedDoc();
+ } else {
+ b = GitilesView.path();
+ }
+ return b.copyFrom(view).setPathPart(path).build().toUrl();
}
@Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java
index 7b3eced..7942253 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/doc/LinkTest.java
@@ -16,7 +16,9 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.base.CharMatcher;
import com.google.gitiles.GitilesView;
+import com.google.gitiles.RootedDocServlet;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
@@ -42,7 +44,7 @@
@Test
public void httpLink() {
- MarkdownToHtml md = new MarkdownToHtml(view, config);
+ MarkdownToHtml md = new MarkdownToHtml(view, config, "index.md");
String url;
url = "http://example.com/foo.html";
@@ -57,7 +59,7 @@
@Test
public void absolutePath() {
- MarkdownToHtml md = new MarkdownToHtml(view, config);
+ MarkdownToHtml md = new MarkdownToHtml(view, config, "index.md");
assertThat(md.href("/")).isEqualTo("/g/repo/+/HEAD/");
assertThat(md.href("/index.md")).isEqualTo("/g/repo/+/HEAD/index.md");
@@ -125,22 +127,77 @@
private MarkdownToHtml file(String path) {
return new MarkdownToHtml(
GitilesView.doc().copyFrom(view).setPathPart(path).build(),
- config);
+ config,
+ path);
}
private MarkdownToHtml repoIndexReadme() {
- return readme(view);
+ return readme(view, "README.md");
}
private MarkdownToHtml revisionReadme() {
- return readme(GitilesView.revision().copyFrom(view).build());
+ return readme(GitilesView.revision().copyFrom(view).build(), "README.md");
}
private MarkdownToHtml treeReadme(String path) {
- return readme(GitilesView.path().copyFrom(view).setPathPart(path).build());
+ GitilesView v = GitilesView.path().copyFrom(view).setPathPart(path).build();
+ String file = CharMatcher.is('/').trimTrailingFrom(path) + "/README.md";
+ return readme(v, file);
}
- private MarkdownToHtml readme(GitilesView v) {
- return new MarkdownToHtml(v, config).setReadme(true);
+ private MarkdownToHtml readme(GitilesView v, String path) {
+ return new MarkdownToHtml(v, config, path);
+ }
+
+ @Test
+ public void rootedDocInRoot() {
+ testRootedDocInRoot(rootedDoc("/", "/index.md"));
+ testRootedDocInRoot(rootedDoc("/index.md", "/index.md"));
+ }
+
+ private void testRootedDocInRoot(MarkdownToHtml md) {
+ assertThat(md.href("setup.md")).isEqualTo("/setup.md");
+ assertThat(md.href("./setup.md")).isEqualTo("/setup.md");
+ assertThat(md.href("./")).isEqualTo("/");
+ assertThat(md.href(".")).isEqualTo("/");
+
+ assertThat(md.href("../")).isEqualTo("#zSoyz");
+ assertThat(md.href("../../")).isEqualTo("#zSoyz");
+ assertThat(md.href("../..")).isEqualTo("#zSoyz");
+ assertThat(md.href("..")).isEqualTo("#zSoyz");
+ }
+
+ @Test
+ public void rootedDocInTree() {
+ testRootedDocInTree(rootedDoc("/doc", "/doc/index.md"));
+ testRootedDocInTree(rootedDoc("/doc/", "/doc/index.md"));
+ testRootedDocInTree(rootedDoc("/doc/index.md", "/doc/index.md"));
+ }
+
+ private void testRootedDocInTree(MarkdownToHtml md) {
+ assertThat(md.href("setup.md")).isEqualTo("/doc/setup.md");
+ assertThat(md.href("./setup.md")).isEqualTo("/doc/setup.md");
+ assertThat(md.href("../setup.md")).isEqualTo("/setup.md");
+ assertThat(md.href("../tech/setup.md")).isEqualTo("/tech/setup.md");
+
+ assertThat(md.href("./")).isEqualTo("/doc");
+ assertThat(md.href(".")).isEqualTo("/doc");
+ assertThat(md.href("../")).isEqualTo("/");
+ assertThat(md.href("..")).isEqualTo("/");
+
+ assertThat(md.href("../../")).isEqualTo("#zSoyz");
+ assertThat(md.href("../..")).isEqualTo("#zSoyz");
+ assertThat(md.href("../../../")).isEqualTo("#zSoyz");
+ assertThat(md.href("../../..")).isEqualTo("#zSoyz");
+ }
+
+ private MarkdownToHtml rootedDoc(String path, String file) {
+ GitilesView view = GitilesView.rootedDoc()
+ .setHostName("gerritcodereview.com")
+ .setServletPath("")
+ .setRevision(RootedDocServlet.BRANCH)
+ .setPathPart(path)
+ .build();
+ return new MarkdownToHtml(view, config, file);
}
}
