Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed page ID for IIS 7 (no semicolon necessary) #84

Merged
merged 1 commit into from Nov 18, 2012

Conversation

borekb
Copy link
Contributor

@borekb borekb commented Feb 27, 2012

No description provided.

…ersions of IIS, semicolon is displayed as is, not URL-encoded.
@splitbrain
Copy link
Collaborator

I wonder if IIS ever was affected by this problem. I only ever tested this with Apache on Windows.

I'd apply this patch but don't like that this needs to be adjusted for every upcoming release of IIS. Eg. the current implementation wouldn't work on IIS 8.

Would be great if someone could check if colons in URLs work in lower versions of IIS. Then we could simply check for the server name.

@borekb
Copy link
Contributor Author

borekb commented May 9, 2012

I was not sure if that block was specifically for IIS (and maybe for some specific version of it) so I added a "safe" modification but I agree, it's not nice to ask for a specific version number.

I can't confirm if DokuWiki runs fine on IIS 6 at its current version but I can definitely confirm that it used to, with colons and not semicolons. I was actually quite surprised when I installed DokuWiki after not using it in ~2 years and there were semicolons everywhere.

@lupo49
Copy link
Contributor

lupo49 commented May 9, 2012

I'll check it for the IIS 6 version.

What should I explicitly check? If the IIS 6 runs fine with this commit applied?

@splitbrain
Copy link
Collaborator

@lupo49 run an unmodified dokuwiki (no additional config settings after install.php) and create a link to a page in a sub namespace. Eg. [[foo:bar]] it should link to foo;bar. Now apply the patch above (without any version checking) and make sure the page got recached. It should now link to foo:bar. Check if this link works or results in an error message.

@selfthinker
Copy link
Collaborator

Or easier: Just check with the elseif completely removed.

@lupo49
Copy link
Contributor

lupo49 commented Jul 6, 2012

Will do in calendar week 28.

@lupo49
Copy link
Contributor

lupo49 commented Jul 14, 2012

If I link to [[foo:bar]] on an IIS 6 (Windows Server 2003) with DokuWiki 2012-07-13 release the links references to http://localhost/dokuwiki/doku.php?id=foo:bar. There's no semicolon, do I have to change something first?

Will check with IIS 7 now.

@lupo49
Copy link
Contributor

lupo49 commented Jul 14, 2012

Same behavior on an IIS 7 (IIS 7.5, Windows Server 2008) with same DW version.

@selfthinker
Copy link
Collaborator

You need to test with $conf['userewrite'] switched on.

@lupo49
Copy link
Contributor

lupo49 commented Jul 14, 2012

Ok, userewrite is set to "DokuWiki internal".

Behavior without the patch:
Link references on IIS6 to: http://localhost/dokuwiki/doku.php/ns1%3Bpage1
Link references on IIS7 to: http://localhost/dokuwiki/doku.php/ns1%3Bpage1

With patch:
Link references on IIS6 to: http://localhost/dokuwiki/doku.php/ns1;page1 (Link is working correctly)
Link references on IIS7 to: http://localhost/dokuwiki/doku.php/ns1:page1 (Link is working correctly)

@selfthinker
Copy link
Collaborator

@lupo49, can you please also test without the version checking like @splitbrain said? That means you can just comment all of the elseif, so only test the if, without anything below.

@lupo49
Copy link
Contributor

lupo49 commented Jul 14, 2012

Works also, both links (II6/IIS7) reference to http://localhost/dokuwiki/doku.php/ns1:page1

Code:

function idfilter($id,$ue=true){
    global $conf;
    if ($conf['useslash'] && $conf['userewrite']){
        $id = strtr($id,':','/');
    }
    if($ue){
        $id = rawurlencode($id);
        $id = str_replace('%3A',':',$id); //keep as colon
        $id = str_replace('%3B',';',$id); //keep as semicolon
        $id = str_replace('%2F','/',$id); //keep as slash
    }
    return $id;
}

@selfthinker
Copy link
Collaborator

Great, thanks.

@selfthinker
Copy link
Collaborator

Just for the record, on my windows setup (I'm using "Uniform Server" 7.0.0 on Windows 7), I get a 403 error without the else case. ("You don't have permission to access /foo:bar on this server.") Judging from Andi's comment, that was the reason why he put it there in the first place. Not sure if this only happens with "unprofessional" servers?

@lupo49
Copy link
Contributor

lupo49 commented Jul 14, 2012

I could set up a remote control tool on the system where the IIS is running, if someone would like to check it again? It would be better to double check before the patch become applied in the stable tree and causes bumpy productive DokuWiki instances.

@splitbrain
Copy link
Collaborator

Judging from the test results it seems only Apache does not allow colons in URLs. Or it might be some webservers allow it, some don't. I guess the safer way is to whitelist servers that allow it. For now that's IIS only. Other servers would need to be tested.

@borekb
Copy link
Contributor Author

borekb commented Oct 17, 2012

Just upgraded to Adora Belle and noticed that my URLs are still being overwritten to the ones with semicolon. FYI, the fix proposed in this pull request still works, maybe it should even be enhanced of the version 8 of IIS that shipped as part of Windows Server 2012 this summer.

@splitbrain splitbrain merged commit 15ae43b into dokuwiki:master Nov 18, 2012
@Klap-in
Copy link
Collaborator

Klap-in commented Jul 13, 2014

What happens with this commit? i cannot find it in the backlog of inc/common.php.

More, i think this issue samuelet/indexmenu#68 is affected by missing
$id = str_replace('%3B',';',$id); //keep as semicolon

Klap-in added a commit that referenced this pull request Jul 15, 2014
Restores vanished changes of #84
splitbrain added a commit that referenced this pull request Apr 9, 2020
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.

None yet

5 participants