librewiki / liberty-skin

Liberty skin
GNU General Public License v3.0
17 stars 49 forks source link

HTML parsing error related in the Navbar #27

Closed ahn9807 closed 4 weeks ago

ahn9807 commented 3 months ago

Hello, thank you very much for creating the Liberty skin.

Currently, there is an issue where the site is not hyperlinked properly due to an HTML parsing error when '=' and '&' are included in the HTML.

For instance, using "https://papago.naver.net/website?locale=ko&source=ko&target=en&url=https://xxx" creates parsing errors at the "=", and due to incorrect usages of the "htmlentities(): function, the result is "https://papago.naver.net/website?".

Since I am not familiar with PHP, I applied a simple patch as follows, but I am concerned about any potential security vulnerabilities.

// LibertyTemplate.php
 foreach ( $split as $key => $value ) {
         $valueArr = explode( '=', trim( $value ) );
         if ( isset( $valueArr[1] ) ) {
                 $newValue = implode( '=', array_slice($valueArr, 1)); // <- Temporary fix for concat hyperlink with "=".
                 $data[$valueArr[0]] = $newValue;
         } else {
                 $data[$types[$key]] = trim( $value );
         }
 }
// LibertyTemplate.php
if ( isset( $data['link'] ) ) {
        // @todo CHECKME: Should this use wfUrlProtocols() or somesuch instead?
        if ( preg_match( '/^((?:(?:http(?:s)?)?:)?\/\/(?:.{4,}))$/i', $data['link'] ) ) {
                $href = htmlentities( $data['link'], ENT_QUOTES, 'UTF-8' );
                $href = $data['link']; // <- Temporary fix for ignoring htmlentitites produce hyperlink with &amp.
        } else {
                $href = str_replace( '%3A', ':', urlencode( $data['link'] ) );
                $href = str_replace( '$1', $href, $wgArticlePath );
        }
} else {
        $href = null;
}

If you could confirm, I will apply the related changes and submit a pull request.

Thank you.

Hoto-Cocoa commented 3 months ago

Hi.

Seems good but second patch will be better If use wfUrlProtocols that mentioned by you, and first patch may not required.

We are welcome to PR, feel free to open.

ahn9807 commented 3 months ago

감사합니다! first patch와 같은 경우는 parsing에러때문에 필요하다고 생각하는데, 수정하는것이 좋아 보입니다. key가 site이고 value가 https://xxx.com/q=asdfasdf 와 같은 링크는 파싱에러가 발생합니다. 확인 부탁드려도 될까요?

아 그리고 wrUrlProtocols와 같은 경우에는, 원래 부터 있던 주석입니다.

Justman100 commented 4 weeks ago

@Hoto-Cocoa

Hoto-Cocoa commented 4 weeks ago

Fixed in #28.