Skip to content

Login Module emitting invalid html. Update default_logout.php (enclose params JRoute parameter in htmlentities)#17368

Closed
LivioCavallo wants to merge 1 commit intojoomla:stagingfrom
LivioCavallo:patch-2
Closed

Login Module emitting invalid html. Update default_logout.php (enclose params JRoute parameter in htmlentities)#17368
LivioCavallo wants to merge 1 commit intojoomla:stagingfrom
LivioCavallo:patch-2

Conversation

@LivioCavallo
Copy link
Copy Markdown
Contributor

@LivioCavallo LivioCavallo commented Jul 31, 2017

Enclose JRoute param in htmlentities to avoid emitting invalid html

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Create some contacts with associated tags
Create a menu item of type tagged-elements of contacts type
Publish a login module on that page.

Expected result

Valid html

Actual result

Invalid html. The login form url contains invalid '[' and ']' chars; th eurl will be similar to (when sef url disabled): /index.php?option=com_tags&view=tag&id[0]=2&types[0]=2&Itemid=nnn

This problem is related to issue "Bug in AbstractUri::buildQuery - invalid HTML emitted ('[' and ']' not encoded in tagged elements list) #21" (joomla-framework/uri#21).

I think the preferred way to solve both problems is solving the above mentioned problem in AbstractUri:buildQuery

Documentation Changes Required

Enclose JRoute param in htmlentities to avoid emitting invalid html
@ghost
Copy link
Copy Markdown

ghost commented Jul 31, 2017

can you please give Test Instuctions?

@ReLater
Copy link
Copy Markdown
Contributor

ReLater commented Jul 31, 2017

Hm, the parameter usesecure is a Yes/No (1/0) parameter.

<field
name="usesecure"
type="radio"
label="MOD_LOGIN_FIELD_USESECURE_LABEL"
description="MOD_LOGIN_FIELD_USESECURE_DESC"
class="btn-group btn-group-yesno"
default="0"
>
<option value="1">JYES</option>
<option value="0">JNO</option>
</field>

So,

htmlentities($params->get('usesecure'))

and this pr don't make sense at all.

Even if you would inject sth. stupid in parameter usesecure. JRoute::_() is sanitizing it with an (int).
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/application/route.php#L73
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/application/route.php#L84

The same argumentation for pr #17367 to close both prs.

@LivioCavallo
Copy link
Copy Markdown
Contributor Author

LivioCavallo commented Jul 31, 2017

To reproduce the problem please do the following:

Create some contacts with associated tags
Create a menu item of type tagged-elements of contacts type
Publish a login module on that page.
The login form on that page will have a url similar to (when sef url disabled): /index.php?option=com_tags&view=tag&id[0]=2&types[0]=2&Itemid=132

As you can see the emitted html is invalid: the query part has in fact illegal characters in it, '[' and ']'; we should let that chars encoded!

This problem is related to issue "Bug in AbstractUri::buildQuery - invalid HTML emitted ('[' and ']' not encoded in tagged elements list) #21" (joomla-framework/uri#21).

I think the preferred way to solve both problems is solving the above mentioned problem in AbstractUri:buildQuery.

The same is for pr #17367

@LivioCavallo
Copy link
Copy Markdown
Contributor Author

Probably mbabker in PR #21 is right: changing AbstractUri:buildQuery is not the best option and it could be a dangerous B/C.

So I think the fix suggested here is the solution.

@ReLater
Copy link
Copy Markdown
Contributor

ReLater commented Aug 1, 2017

So I think the fix suggested here is the solution.

In parts:
$params->get('usesecure')
results in
'1' or '0'.
It stands for "Use SSL (https)" "yes" or "no". Just a switch.

Thus your
htmlentities($params->get('usesecure'))
results in
htmlentities('1') or htmlentities('0')
that results in
'1' or '0'.

@LivioCavallo
Copy link
Copy Markdown
Contributor Author

Yes, you obviously are absolutely right!
I detected the problem here (in login form) but it does not originate here and in no way this is a fix.

Sorry, my confusion deriving from a J!3.4.3 workaround...

The problem remains (tagged emelents menu item emits invalid html)

@LivioCavallo
Copy link
Copy Markdown
Contributor Author

I close this PR and #17367

@LivioCavallo LivioCavallo deleted the patch-2 branch August 1, 2017 12: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.

3 participants