? Success

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
23 Jul 2015

This improvement lets check easily if the xml is correct by displaying a link to its location under the site name.

After patch one should get:
screen shot 2015-07-23 at 17 29 08

@nikosdion

avatar infograf768 infograf768 - open - 23 Jul 2015
avatar infograf768 infograf768 - change - 23 Jul 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 23 Jul 2015
Labels Added: ?
avatar infograf768 infograf768 - change - 23 Jul 2015
Priority Medium Low
Easy No Yes
avatar infograf768 infograf768 - change - 23 Jul 2015
Category Components
avatar infograf768 infograf768 - change - 23 Jul 2015
Category Components Administration Components
avatar Fedik
Fedik - comment - 23 Jul 2015

looks cool :wink:
maybe add target="_blank" for open the link in the new tab/window?

avatar infograf768
infograf768 - comment - 23 Jul 2015

Added :+1:

avatar nikosdion
nikosdion - comment - 23 Jul 2015

@mbabker @wilsonge Do you think we should add some escaping to the displayed data and the link?

avatar mbabker
mbabker - comment - 23 Jul 2015

Might be a good idea.

avatar infograf768
infograf768 - comment - 23 Jul 2015

Something like ?:

                        <td>
                            <label for="cb<?php echo $i; ?>">
                                <?php echo $item->update_site_name; ?>
                                <br /><span class="small"><a href="<?php echo $item->location; ?>" target="_blank">
                                <?php echo $this->escape($item->location); ?></a></span>
                            </label>
                        </td>
avatar infograf768
infograf768 - comment - 23 Jul 2015
avatar Fedik
Fedik - comment - 23 Jul 2015

test good, for me :smile:

avatar Fedik Fedik - test_item - 23 Jul 2015 - Tested successfully
avatar Bakual
Bakual - comment - 23 Jul 2015

Escaping the link when you use it as link doesn't make sense. But if you display it it makes sense. Your code looks fine and is the same like in com_installer where the display part is escaped as well :smile:

avatar nikosdion
nikosdion - comment - 23 Jul 2015

Escaping the link when you use it as link doesn't make sense

You are right. Thinking about it again, putting malicious content in that field would require the Super User to install an extension with a malicious update server URL. But if a hacker can get you to install their software on your server it's game over anyway. Brain fart. Please ignore me.

avatar Bakual
Bakual - comment - 23 Jul 2015

I had the same thought and came to same conclusion :)

avatar infograf768
infograf768 - comment - 24 Jul 2015

Escaped display part. I guess this can now go RTC. Thanks for testing.

avatar Bakual Bakual - change - 24 Jul 2015
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2015-07-24 06:33:18
Closed_By Bakual
avatar Bakual Bakual - close - 24 Jul 2015
avatar Bakual Bakual - reference | 9a458a0 - 24 Jul 15
avatar Bakual Bakual - merge - 24 Jul 2015
avatar Bakual Bakual - close - 24 Jul 2015
avatar Bakual Bakual - change - 24 Jul 2015
Milestone Added:
avatar infograf768 infograf768 - head_ref_deleted - 24 Jul 2015

Add a Comment

Login with GitHub to post a comment