Fix relative and absolute markdown links within repo [link](/a) should resolve relative to the top of the repository, not the top of the host. [link](../../C) from within A/B/ should correctly resolve to /C and not a broken link. Update DocServletTest to fix tests previously broken by anchor hyperlink widgets (263d674ee88311b3b). Change-Id: I036c47f22a52d82919f7472c751db766b88d23f7
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java index aac9f08..54cf377 100644 --- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java +++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -323,6 +323,7 @@ break; case PATH: case SHOW: + case DOC: checkPath(); break; case DIFF: @@ -337,8 +338,6 @@ case BLAME: checkBlame(); break; - case DOC: - checkDoc(); case ROOTED_DOC: checkRootedDoc(); break; @@ -401,10 +400,6 @@ checkPath(); } - private void checkDoc() { - checkRevision(); - } - private void checkRootedDoc() { checkView(hostName != null, "missing hostName on %s view", type); checkView(servletPath != null, "missing hostName on %s view", type);
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 cd67d9c..7b7f974 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
@@ -67,7 +67,7 @@ // Generation of ETag logic. Bump this only if DocServlet logic changes // significantly enough to impact cached pages. Soy template and source // files are automatically hashed as part of the ETag. - private static final int ETAG_GEN = 2; + private static final int ETAG_GEN = 3; public DocServlet(GitilesAccess.Factory accessFactory, Renderer renderer) { super(renderer, accessFactory);
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 4138f0a..eb602ea 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
@@ -23,6 +23,7 @@ import com.google.gitiles.doc.html.HtmlBuilder; import com.google.template.soy.data.SanitizedContent; import com.google.template.soy.shared.restricted.EscapingConventions.FilterImageDataUri; +import com.google.template.soy.shared.restricted.EscapingConventions.FilterNormalizeUri; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.util.StringUtils; @@ -372,22 +373,48 @@ private String href(String url) { if (HtmlBuilder.isValidHttpUri(url)) { return url; - } - if (MarkdownUtil.isAbsolutePathToMarkdown(url)) { + } else if (MarkdownUtil.isAbsolutePathToMarkdown(url)) { return GitilesView.doc().copyFrom(view).setPathPart(url).build().toUrl(); + } else if (url.startsWith("/")) { + return GitilesView.show().copyFrom(view).setPathPart(url).build().toUrl(); + } else if (!url.startsWith("../") && !url.startsWith("./")) { + return url; } - if (readme && !url.startsWith("../") && !url.startsWith("./")) { - String dir = ""; - if (view.getPathPart() != null && view.getPathPart().endsWith("/")) { - dir = view.getPathPart(); - } else if (view.getPathPart() != null) { - dir = view.getPathPart() + '/'; + + String dir = Strings.nullToEmpty(view.getPathPart()); + if (!readme) { + int slash = dir.lastIndexOf('/'); + dir = slash < 0 ? "" : dir.substring(0, slash); + } + while (!url.isEmpty()) { + if (url.startsWith("./")) { + url = url.substring(2); + } else if (url.startsWith("../")) { + if (dir.isEmpty()) { + return FilterNormalizeUri.INSTANCE.getInnocuousOutput(); + } + url = url.substring(3); + + int slash = dir.lastIndexOf('/'); + if (slash >= 0) { + dir = dir.substring(0, slash); + } else { + dir = ""; + break; + } + } else { + break; } - return GitilesView.path().copyFrom(view) - .setPathPart(dir + url) - .build().toUrl(); } - return url; + + if (url.isEmpty()) { + return FilterNormalizeUri.INSTANCE.getInnocuousOutput(); + } + + GitilesView.Builder dest = url.endsWith(".md") + ? GitilesView.doc() + : GitilesView.show(); + return dest.copyFrom(view).setPathPart(dir + url).build().toUrl(); } @Override
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java index dffe4f2..56b7227 100644 --- a/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java +++ b/gitiles-servlet/src/test/java/com/google/gitiles/doc/DocServletTest.java
@@ -69,7 +69,7 @@ String html = buildHtml("/repo/+doc/master/README.md"); assertTrue(html.contains("<title>" + title + "</title>")); - assertTrue(html.contains("<h1>" + title + "</h1>")); + assertTrue(html.contains(title + "</h1>")); assertTrue(html.contains("<a href=\"" + url + "\">Markdown</a>")); } @@ -91,7 +91,9 @@ assertTrue(html.contains("<h2>page</h2>")); assertTrue(html.contains("<li><a href=\"index.md\">Home</a></li>")); assertTrue(html.contains("<li><a href=\"README.md\">README</a></li>")); - assertTrue(html.contains("<h1>page</h1>")); + assertTrue(html.contains("<h1>" + + "<a class=\"h\" name=\"page\" href=\"#page\"><span></span></a>" + + "page</h1>")); } @Test @@ -106,7 +108,7 @@ .create(); String html = buildHtml("/repo/+doc/master/"); - assertTrue(html.contains("<h1>B. Ad</h1>")); + assertTrue(html.contains("B. Ad</h1>")); assertTrue(html.contains("Non-HTML is fine.")); assertFalse(html.contains("window.alert")); @@ -121,8 +123,12 @@ .add("index.md", markdown) .create(); String html = buildHtml("/repo/+doc/master/"); - assertTrue(html.contains("<a name=\"debug\"><h1>Section</h1></a>")); - assertTrue(html.contains("<a name=\"old-school\"><h1>Other</h1></a>")); + assertTrue(html.contains("<h1>" + + "<a class=\"h\" name=\"debug\" href=\"#debug\"><span></span></a>" + + "Section</h1>")); + assertTrue(html.contains("<h1>" + + "<a class=\"h\" name=\"old-school\" href=\"#old-school\"><span></span></a>" + + "Other</h1>")); } @Test @@ -136,6 +142,26 @@ assertTrue(html.contains("Incomplete <html is literal.")); } + @Test + public void relativeLink() throws Exception { + repo.branch("master").commit() + .add("A/B/README.md", "[c](../../C)") + .create(); + + String html = buildHtml("/repo/+doc/master/A/B/README.md"); + assertTrue(html.contains("<a href=\"/b/repo/+show/master/C\">c</a>")); + } + + @Test + public void absoluteLink() throws Exception { + repo.branch("master").commit() + .add("README.md", "[c](/x)") + .create(); + + String html = buildHtml("/repo/+doc/master/README.md"); + assertTrue(html.contains("<a href=\"/b/repo/+show/master/x\">c</a>")); + } + private String buildHtml(String pathAndQuery) throws Exception { TestViewFilter.Result res = service(pathAndQuery); FakeHttpServletResponse http = res.getResponse();