fix #1264 improve urlLink build

src/main/java/org/codelibs/fess/helper/UserAgentHelper.java

@@ -15,8 +15,6 @@
  */
 package org.codelibs.fess.helper;
 
-import javax.servlet.http.HttpServletRequest;
-
 import org.lastaflute.web.util.LaRequestUtil;
 
 public class UserAgentHelper {
@@ -25,29 +23,30 @@ public class UserAgentHelper {
     private static final String USER_AGENT_TYPE = "ViewHelper.UserAgent";
 
     public UserAgentType getUserAgentType() {
-        final HttpServletRequest request = LaRequestUtil.getRequest();
-        UserAgentType uaType = (UserAgentType) request.getAttribute(USER_AGENT_TYPE);
-        if (uaType == null) {
-            final String userAgent = request.getHeader(USER_AGENT);
-            if (userAgent != null) {
-                if (userAgent.indexOf("MSIE") >= 0 || userAgent.indexOf("Trident") >= 0) {
-                    uaType = UserAgentType.IE;
-                } else if (userAgent.indexOf("Firefox") >= 0) {
-                    uaType = UserAgentType.FIREFOX;
-                } else if (userAgent.indexOf("Chrome") >= 0) {
-                    uaType = UserAgentType.CHROME;
-                } else if (userAgent.indexOf("Safari") >= 0) {
-                    uaType = UserAgentType.SAFARI;
-                } else if (userAgent.indexOf("Opera") >= 0) {
-                    uaType = UserAgentType.OPERA;
-                }
-            }
+        return LaRequestUtil.getOptionalRequest().map(request -> {
+            UserAgentType uaType = (UserAgentType) request.getAttribute(USER_AGENT_TYPE);
             if (uaType == null) {
-                uaType = UserAgentType.OTHER;
+                final String userAgent = request.getHeader(USER_AGENT);
+                if (userAgent != null) {
+                    if (userAgent.indexOf("MSIE") >= 0 || userAgent.indexOf("Trident") >= 0) {
+                        uaType = UserAgentType.IE;
+                    } else if (userAgent.indexOf("Firefox") >= 0) {
+                        uaType = UserAgentType.FIREFOX;
+                    } else if (userAgent.indexOf("Chrome") >= 0) {
+                        uaType = UserAgentType.CHROME;
+                    } else if (userAgent.indexOf("Safari") >= 0) {
+                        uaType = UserAgentType.SAFARI;
+                    } else if (userAgent.indexOf("Opera") >= 0) {
+                        uaType = UserAgentType.OPERA;
+                    }
+                }
+                if (uaType == null) {
+                    uaType = UserAgentType.OTHER;
+                }
+                request.setAttribute(USER_AGENT_TYPE, uaType);
             }
-            request.setAttribute(USER_AGENT_TYPE, uaType);
-        }
-        return uaType;
+            return uaType;
+        }).orElse(UserAgentType.OTHER);
     }
 
     public enum UserAgentType {

src/main/java/org/codelibs/fess/helper/ViewHelper.java

@@ -180,26 +180,31 @@ public class ViewHelper {
     }
 
     public String getUrlLink(final Map<String, Object> document) {
-        // file protocol
-        String url = DocumentUtil.getValue(document, "url", String.class);
+        final FessConfig fessConfig = ComponentUtil.getFessConfig();
+        String url = DocumentUtil.getValue(document, fessConfig.getIndexFieldUrl(), String.class);
 
-        if (url == null) {
-            // TODO should redirect to a invalid page?
-            return "#";
+        if (StringUtil.isBlank(url)) {
+            return "#not-found-" + DocumentUtil.getValue(document, fessConfig.getIndexFieldDocId(), String.class);
         }
 
-        final boolean isFileUrl = url.startsWith("smb:") || url.startsWith("ftp:");
+        final boolean isSmbUrl = url.startsWith("smb:");
+        final boolean isFtpUrl = url.startsWith("ftp:");
+        final boolean isSmbOrFtpUrl = isSmbUrl || isFtpUrl;
 
         // replacing url with mapping data
-        url = pathMappingHelper.replaceUrl(url);
+        if (pathMappingHelper != null) {
+            url = pathMappingHelper.replaceUrl(url);
+        }
+
+        final boolean isHttpUrl = url.startsWith("http:") || url.startsWith("https:");
 
-        if (url.startsWith("smb:")) {
+        if (isSmbUrl) {
             url = url.replace("smb:", "file:");
-        } else if (url.startsWith("ftp:")) {
-            url = url.replace("ftp:", "file:");
         }
 
-        if (url.startsWith("http:") && isFileUrl) {
+        if (isHttpUrl && isSmbOrFtpUrl) {
+            //  smb/ftp->http
+            // encode
             final StringBuilder buf = new StringBuilder(url.length() + 100);
             for (final char c : url.toCharArray()) {
                 if (CharUtil.isUrlChar(c)) {
@@ -208,89 +213,96 @@ public class ViewHelper {
                     try {
                         buf.append(URLEncoder.encode(String.valueOf(c), urlLinkEncoding));
                     } catch (final UnsupportedEncodingException e) {
-                        // NOP
+                        buf.append(c);
                     }
                 }
             }
             url = buf.toString();
         } else if (url.startsWith("file:")) {
-
-            final int pos = url.indexOf(':', 5);
-            final boolean isLocalFile = pos > 0 && pos < 12;
-
-            final UserAgentType ua = userAgentHelper.getUserAgentType();
-            switch (ua) {
-            case IE:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.ie", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.ie", "file://"));
-                }
-                break;
-            case FIREFOX:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.firefox", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.firefox", "file://///"));
-                }
-                break;
-            case CHROME:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.chrome", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.chrome", "file://"));
-                }
-                break;
-            case SAFARI:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.safari", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.safari", "file:////"));
-                }
-                break;
-            case OPERA:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.opera", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.opera", "file://"));
-                }
-                break;
-            default:
-                if (isLocalFile) {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.other", "file://"));
-                } else {
-                    url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.other", "file://"));
-                }
-                break;
-            }
+            // file, smb/ftp->http
+            url = updateFileProtocol(url);
 
             if (encodeUrlLink) {
                 return appendQueryParameter(document, url);
-            } else {
-                final String urlLink = appendQueryParameter(document, url).replace("+", "%2B");
-                final String[] values = urlLink.split("#");
-                final StringBuilder buf = new StringBuilder(urlLink.length());
+            }
+
+            // decode
+            if (!isSmbOrFtpUrl) {
+                // file
                 try {
-                    buf.append(URLDecoder.decode(values[0], urlLinkEncoding));
-                    for (int i = 1; i < values.length - 1; i++) {
-                        buf.append('#').append(URLDecoder.decode(values[i], urlLinkEncoding));
+                    url = URLDecoder.decode(url.replace("+", "%2B"), urlLinkEncoding);
+                } catch (Exception e) {
+                    if (logger.isDebugEnabled()) {
+                        logger.warn("Failed to decode " + url, e);
                     }
-                } catch (final Exception e) {
-                    throw new FessSystemException("Unsupported encoding: " + urlLinkEncoding, e);
-                }
-                if (values.length > 1) {
-                    buf.append('#').append(values[values.length - 1]);
                 }
-                return buf.toString();
             }
         }
+        // http, ftp
+        // nothing
 
         return appendQueryParameter(document, url);
     }
 
+    protected String updateFileProtocol(String url) {
+        final int pos = url.indexOf(':', 5);
+        final boolean isLocalFile = pos > 0 && pos < 12;
+
+        final UserAgentType ua = userAgentHelper.getUserAgentType();
+        switch (ua) {
+        case IE:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.ie", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.ie", "file://"));
+            }
+            break;
+        case FIREFOX:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.firefox", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.firefox", "file://///"));
+            }
+            break;
+        case CHROME:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.chrome", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.chrome", "file://"));
+            }
+            break;
+        case SAFARI:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.safari", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.safari", "file:////"));
+            }
+            break;
+        case OPERA:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.opera", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.opera", "file://"));
+            }
+            break;
+        default:
+            if (isLocalFile) {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.winlocal.other", "file://"));
+            } else {
+                url = url.replaceFirst("file:/+", systemProperties.getProperty("file.protocol.other", "file://"));
+            }
+            break;
+        }
+        return url;
+    }
+
     protected String appendQueryParameter(final Map<String, Object> document, final String url) {
         final FessConfig fessConfig = ComponentUtil.getFessConfig();
         if (fessConfig.isAppendQueryParameter()) {
+            if (url.indexOf('#') >= 0) {
+                return url;
+            }
+
             final String mimetype = DocumentUtil.getValue(document, fessConfig.getIndexFieldMimetype(), String.class);
             if (StringUtil.isNotBlank(mimetype)) {
                 if ("application/pdf".equals(mimetype)) {

src/test/java/org/codelibs/fess/helper/ViewHelperTest.java

@@ -15,9 +15,15 @@
  */
 package org.codelibs.fess.helper;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.codelibs.core.io.FileUtil;
+import org.codelibs.core.misc.DynamicProperties;
+import org.codelibs.fess.es.config.exentity.PathMapping;
 import org.codelibs.fess.mylasta.direction.FessConfig;
 import org.codelibs.fess.unit.UnitFessTestCase;
 import org.codelibs.fess.util.ComponentUtil;
@@ -31,6 +37,109 @@ public class ViewHelperTest extends UnitFessTestCase {
         viewHelper.init();
     }
 
+    public void test_getUrlLink() throws IOException {
+        ComponentUtil.setFessConfig(new FessConfig.SimpleImpl() {
+            private static final long serialVersionUID = 1L;
+
+            public boolean isAppendQueryParameter() {
+                return false;
+            }
+
+            public String getIndexFieldUrl() {
+                return "url";
+            }
+
+            public String getIndexFieldDocId() {
+                return "doc_id";
+            }
+        });
+
+        // http
+        assertUrlLink("http://www.codelibs.org/", //
+                "http://www.codelibs.org/");
+        assertUrlLink("http://www.codelibs.org/あ", //
+                "http://www.codelibs.org/あ");
+        assertUrlLink("http://www.codelibs.org/%E3%81%82", //
+                "http://www.codelibs.org/%E3%81%82");
+        assertUrlLink("http://www.codelibs.org/%z", //
+                "http://www.codelibs.org/%z");
+        assertUrlLink("http://www.codelibs.org/?a=1&b=2", //
+                "http://www.codelibs.org/?a=1&b=2");
+
+        // https
+        assertUrlLink("https://www.codelibs.org/", //
+                "https://www.codelibs.org/");
+        assertUrlLink("https://www.codelibs.org/あ", //
+                "https://www.codelibs.org/あ");
+        assertUrlLink("https://www.codelibs.org/%E3%81%82", //
+                "https://www.codelibs.org/%E3%81%82");
+        assertUrlLink("https://www.codelibs.org/%z", //
+                "https://www.codelibs.org/%z");
+        assertUrlLink("https://www.codelibs.org/?a=1&b=2", //
+                "https://www.codelibs.org/?a=1&b=2");
+
+        // ftp
+        assertUrlLink("ftp://www.codelibs.org/", //
+                "ftp://www.codelibs.org/");
+        assertUrlLink("ftp://www.codelibs.org/あ", //
+                "ftp://www.codelibs.org/あ");
+        assertUrlLink("ftp://www.codelibs.org/%E3%81%82", //
+                "ftp://www.codelibs.org/%E3%81%82");
+        assertUrlLink("ftp://www.codelibs.org/%z", //
+                "ftp://www.codelibs.org/%z");
+
+        viewHelper.userAgentHelper = new UserAgentHelper() {
+            public UserAgentType getUserAgentType() {
+                return UserAgentType.IE;
+            }
+        };
+        File tempFile = File.createTempFile("test", ".properties");
+        FileUtil.writeBytes(tempFile.getAbsolutePath(), new byte[0]);
+        tempFile.deleteOnExit();
+        viewHelper.systemProperties = new DynamicProperties(tempFile);
+
+        // file
+        assertUrlLink("file:/home/taro/test.txt", //
+                "file://home/taro/test.txt");
+        assertUrlLink("file:/home/taro/あ.txt", //
+                "file://home/taro/あ.txt");
+        assertUrlLink("file:/home/taro/%E3%81%82.txt", //
+                "file://home/taro/あ.txt");
+
+        // smb->file
+        assertUrlLink("smb:/home/taro/test.txt", //
+                "file://home/taro/test.txt");
+        assertUrlLink("smb:/home/taro/あ.txt", //
+                "file://home/taro/あ.txt");
+        assertUrlLink("smb:/home/taro/%E3%81%82.txt", //
+                "file://home/taro/%E3%81%82.txt");
+
+        PathMapping pathMapping = new PathMapping();
+        pathMapping.setRegex("ftp:");
+        pathMapping.setReplacement("file:");
+        viewHelper.pathMappingHelper = new PathMappingHelper();
+        viewHelper.pathMappingHelper.cachedPathMappingList = new ArrayList<>();
+        viewHelper.pathMappingHelper.cachedPathMappingList.add(pathMapping);
+        // ftp->file
+        assertUrlLink("ftp:/home/taro/test.txt", //
+                "file://home/taro/test.txt");
+        assertUrlLink("ftp:/home/taro/あ.txt", //
+                "file://home/taro/あ.txt");
+        assertUrlLink("ftp:/home/taro/%E3%81%82.txt", //
+                "file://home/taro/%E3%81%82.txt");
+
+        assertUrlLink(null, "#not-found-docId");
+        assertUrlLink("", "#not-found-docId");
+        assertUrlLink(" ", "#not-found-docId");
+    }
+
+    private void assertUrlLink(String url, String expected) {
+        Map<String, Object> doc = new HashMap<>();
+        doc.put("doc_id", "docId");
+        doc.put("url", url);
+        assertEquals(expected, viewHelper.getUrlLink(doc));
+    }
+
     public void test_replaceHighlightQueries() {
         String text;
         String[] queries;