Skip to content

Fixed the wrong URL #5070

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

Merged
merged 1 commit into from
Nov 26, 2016
Merged

Fixed the wrong URL #5070

merged 1 commit into from
Nov 26, 2016

Conversation

demis-palma
Copy link
Contributor

@demis-palma demis-palma commented Nov 26, 2016

The preview action while editing a post, points to a malformed URL (half SEF, half non-sef), which is not guaranteed to be parsed correctly by Joomla, and alerts some Web Application Firewall since the php file called (/forum/5-support/topic/index.php) does not exists on the filesystem (see below).

Given that the topic URL is http:// site.com/forum/5-support/topic/create.html
and the preview URL is relative index.php?option=com_kunena&view=topic&layout=edit&format=raw
then the actual preview URL will be /forum/5-support/topic/index.php?option=com_kunena&view=topic&layout=edit&format=raw which is wrong.

In addition, the value calculated for "kpreview_url" was never used.

@810
Copy link
Member

810 commented Nov 26, 2016

i had changed this because suffix was breaking the preview. Did you also test it with suffix?

@demis-palma
Copy link
Contributor Author

I'm sorry, I'm not sure to have understood. What suffix? Could you explain further?

@810
Copy link
Member

810 commented Nov 26, 2016

The joomla url suffix, like .html

We had a problem with showing the preview, with suffix turned on the joomla global configuration.

@demis-palma
Copy link
Contributor Author

Well, for sure there is a problem with old Joomla 1.6 - 2.5 websites updated to Joomla 3, since they have a statement in their .htaccess which is incompatible with some Kunena URLS, especially the upload:
RewriteCond %{REQUEST_URI} /component/|(/[^.]*|\.(php|html?|feed|pdf|vcf|raw))$ [NC]
The statement has been removed in Joomla 3, but old websites often retain their old .htaccess.

That being said, in my opinion, both the code before and after this patch work great regardless of SEF options, because in both cases it points to a non SEF URL.
The problem that this patch wants to solve is that the URL is not completely SEF, still not completely non-SEF. It's a hybrid.
As you can see, it starts with a virtual directory /forum/5-support/topic/... but it ends with a php suffix, instead of the suffix defined in the SEF options.

To conclude, I think that with this patch, the preview should work with every combination of SEF settings, but I haven't tried yet.
Do you want that I make some tests with all the possible SEF settings?

@810 810 added this to the 5.0.4 milestone Nov 26, 2016
@810
Copy link
Member

810 commented Nov 26, 2016

Before:
-suffix on and on reply
http://localhost:8081/forum/welcome-mat/1-welcome-to-kunena/reply/index.php?option=com_kunena&view=topic&layout=edit&format=raw
- suffix on and new topic
http://localhost:8081/forum/index.php?option=com_kunena&view=topic&layout=edit&format=raw

After patch:
-suffix on and on reply
http://localhost:8081/forum/welcome-mat/1-welcome-to-kunena/reply/1.html
- suffix on and new topic
http://localhost:8081/forum/newtopic.html

Before:
-suffix off and on reply
http://localhost:8081/forum/welcome-mat/1-welcome-to-kunena/reply/index.php?option=com_kunena&view=topic&layout=edit&format=raw
- suffix off and new topic
http://localhost:8081/forum/index.php?option=com_kunena&view=topic&layout=edit&format=raw

After patch:
-suffix off and on reply
http://localhost:8081/forum/welcome-mat/1-welcome-to-kunena/reply/1
- suffix off and new topic
http://localhost:8081/forum/newtopic

@810 810 merged commit 5dbe20e into Kunena:K5.0 Nov 26, 2016
810 added a commit that referenced this pull request Nov 26, 2016
@810
Copy link
Member

810 commented Nov 26, 2016

@demis-palma thnx, i have also fixed rating and avatar gallery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants