Skip to content

Do not encode targetUrl as it creates an issue when the home url contain a '/'#29

Closed
seemantr wants to merge 1 commit intoFransBouma:masterfrom
FlexSearch:bug-homelink
Closed

Do not encode targetUrl as it creates an issue when the home url contain a '/'#29
seemantr wants to merge 1 commit intoFransBouma:masterfrom
FlexSearch:bug-homelink

Conversation

@seemantr
Copy link
Copy Markdown
Contributor

@seemantr seemantr commented Mar 5, 2016

This is a corner case which only happens when the __index page is under a folder like in the below case where all my paths are under docs folder. The url which gets generated (http://localhost:5000/docs%2foverview.html) has / replaced with %2f which changes the path.

{
    "Name": "FlexSearch",
    "Source": ".",
    "Destination": "..\\build",
    "Theme": "Default",
    "DefaultExtension": "html",
    "Pages": {
        "__index": "docs/overview.md",
        "FAQ": "docs/faq.md",
        "Getting Started": {
            "__index": "docs/getting-started.md",
            "Essential Path": {
                "Installing FlexSearch": "docs/server-setup/installing.md",
                "Setting up demo index": "docs/demo-index/setting-up-demo-index.md",
                "Search UI": "docs/demo-index/search-ui.md",
                "API Basics": "docs/rest/api-basics.md"
            }.....

@FransBouma FransBouma changed the title Do not encode targetUrl as it creates an issue when the home url contain a '\' Do not encode targetUrl as it creates an issue when the home url contain a '/' Mar 6, 2016
@FransBouma
Copy link
Copy Markdown
Owner

The problem with removing the encode call is that any characters that need to be encoded are left as-is, including spaces.

Isn't a better solution to use System.Uri.EscapeUriString() instead? It will leave / and other url characters alone, but will encode spaces.
bla/bla something.htm will become:
bla/bla%20something.htm
which is what we need :)

I'll change the UrlEncode with Uri.EscapeUriString().

(I didn't look into properly escaping this through the Uri class, I read this morning the HttpUtility uses Uri under the hood so I looked at that class and it's a better fit :))

FransBouma added a commit that referenced this pull request Mar 6, 2016
@FransBouma
Copy link
Copy Markdown
Owner

Closed (fixed by better method)

@FransBouma FransBouma closed this Mar 6, 2016
@seemantr seemantr deleted the bug-homelink branch March 7, 2016 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants