Skip to content
This repository has been archived by the owner on Mar 8, 2021. It is now read-only.

Adding the 6G Firewall stops the site tree expanding #1031

Closed
markryder opened this issue Dec 1, 2016 · 23 comments
Closed

Adding the 6G Firewall stops the site tree expanding #1031

markryder opened this issue Dec 1, 2016 · 23 comments

Comments

@markryder
Copy link

Adding the 6G firewall htaccess to an Evo site stops the site tree in the manager from expanding
If you click just says
Loading Site Tree......
and does not expand.

Does anyone know which rule this is?

@markryder markryder changed the title Adding the 6G Firewall stops the tree Adding the 6G Firewall stops the site tree expanding Dec 1, 2016
@markryder
Copy link
Author

I have worked out which line triggers the error, just not smart enough to change it so that it still protects OK but lets the tree work

@chrille38317
Copy link

And which line is it? I can reproduce it.

@markryder
Copy link
Author

commenting out line 14 lets the site tree work
The issue I think is there is a pipe at the end of the URL called when you expand the tree

@chrille38317
Copy link

OK, I've removed || at the end of the matching string in brackets. This was it. Now, I also hope, that this wasn't critical. ;)

@lukwe
Copy link

lukwe commented Dec 1, 2016

Ok I confirm both the 'Loading site tree' issue with the full 6g code inside the original modx .htaccess, and its solution by commenting out the whole line 14:

RewriteCond %{QUERY_STRING} (\\|\.\.\.|\.\./|~||<|>||) [NC,OR]`

I'd like to only exclude the '||' pipe as Chrille38317 is saying, but I'm not quite sure about hot to do it (?!)

@chrille38317
Copy link

chrille38317 commented Dec 1, 2016

Change

RewriteCond %{QUERY_STRING} (\\|\.\.\.|\.\./|~|`|<|>|\|) [NC,OR]

to

RewriteCond %{QUERY_STRING} (\\|\.\.\.|\.\./|~|`|<|>) [NC,OR]

@lukwe
Copy link

lukwe commented Dec 1, 2016

I actually see '||' there, i.e. the first '|' is an OR separator, and the backslashed '|' is the next (last) query string to match, that's why i did not see it as '||', Thanks anyhow!

BTW, the Manager's 'home' in the main frame is missing both with line 14 commented and with its re-edited version: is it so for everyone ?

@markryder
Copy link
Author

markryder commented Dec 1, 2016

Not doing if for me, still getting 'Loading site tree' with the new line replacing the old one but OK when commented out

@nick0
Copy link

nick0 commented Dec 1, 2016

Thanks for this @chrille38317, your code fixed it for me. Working as expected now. Cheers

@markryder
Copy link
Author

Hi nick
Interesting, what version did it work on, must have missed something

@risingisland
Copy link

risingisland commented Dec 1, 2016

I just made the change, and its working fine for me on v1.0.15

edit: strange, on another site with the same version, it works fine without editing that line?

@matdave
Copy link

matdave commented Dec 1, 2016

I've been working on some changes to 6G since testing on about 25 Evo sites. Here is the version that is so far not causing issues with custom queries: https://bitbucket.org/snippets/ideabank/776Kz

@nick0
Copy link

nick0 commented Dec 2, 2016

Hi Mark,
The 3 sites I tested it on are all 1.0.15 (patched).
Thanks for pointing out the 6G site tree expansion issue - I didn't realise it didn't work until you posted.

Hi @matdave ,
Just wondering how many changes you made in your modified version?

@matdave
Copy link

matdave commented Dec 5, 2016

I just made another little one. Now I'm mostly just eliminating stuff being filtered due to client errors, e.g. spaces in file names.

The big thing was removing 403's when there are commas or pipes in the request string, as those are common delimiters used on requests in MODX.

@risingisland
Copy link

risingisland commented Dec 7, 2016

@matdave
Would you mind explaining what the changes your version effectively do?
For example, why the commas were removed from ([a-z0-9]{2000}), etc.
This would be a big help for those of us less familiar with .htaccess rules.

@matdave
Copy link

matdave commented Dec 7, 2016

Changes started out being primarily for delimiters and URLS containing commas. A lot of processes combine items into a delimited string during the GET request and then explode it into an array later, e.g. the site tree delimits all expanded items with a pipe (1|22|46).

The rest of the changes have been geared towards URL common URL string abnormalities, like spaces in file names, or commas. Every URI request is really a GET request against the index.php file, example.com/some/uri/pattern is translated to example.com/index.php?q=some/uri/pattern. This is why I've made additional changes based on what is valid for people to put in an alias.

@markryder
Copy link
Author

Thanks that makes sense
I discovered via a revo site with 6g added that a comma is OK both in an alias and in an image name
Told the client to change the names, but had never considered a comma in an alias would be accepted

@nick0
Copy link

nick0 commented Dec 9, 2016

I added the 6G htaccess mod to an Evo 1.0.15 (patched) site that has a number of images and pdfs with spaces in the folder and file names.

eg "image 27 cropped.jpg"

This has caused a number of images and files to now not be shown (eg the links are broken).

Without having to manually change each of these names, does anyone know which bit in the 6G code relates to spaces so that I can remove that bit for this site only? I cant see a space in the 6G so I am a little lost. Thanks so much.

@matdave
Copy link

matdave commented Dec 9, 2016

Did you use the one from the 6G site or the modified version I linked above?

@Cipa
Copy link

Cipa commented Dec 9, 2016

In my case I also removed the % from the list of not allowed characters, I need some urls the have %20 in them as space, although maybe adding %20 to the allow list might have been better(was esier to remove)

@nick0
Copy link

nick0 commented Dec 13, 2016

Hi @matdave, I was using the Perishable Press one.
But have since change to your line:
RedirectMatch 403 (?i)(~||<|>|:|;|{|}|[|])`
Now files with spaces work aok.

Thanks @Cipa, Great tip. I just did that based on your advice.
There seemed to be a number of pdfs on this site that just had spaces rather than %20 so it still wasn't quite working right until I used that line from the @matdave version.

Many thanks to you both,

@modxuser
Copy link

@markryder If this issue is resolved can you close it please, if you are not sure how to, take a look here

@markryder
Copy link
Author

Hi modxuser, sorry missed that.
Thank you matdave your version works great!!

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

No branches or pull requests

8 participants