Force HTTPS for Tibia forums

[mathiasbynens]

http://forum.tibia.com/forum/* is now available over HTTPS through https://secure.tibia.com/forum/*.

[ivysrono]

• change rule name to tibia.com
• change filename to tibia.com.xml

add target:

• forum.tibia.com
• forum.test.tibia.com
• secure.tibia.com
• secure.test.tibia.com

add rule

• add <rule from="^http:" to="https:" /> for secure.(test.)?tibia.com

[mathiasbynens]

@ivysrono Thanks for the feedback! Done.

[ivysrono]

@mathiasbynens I have updated TODO list.

[mathiasbynens]

@ivysrono secure.(test.)?tibia.com only work correctly over HTTPS — over HTTP, they just return an error page, so there shouldn’t be any links to their HTTP version in the first place. As such, do you still think <rule from="^http:" to="https:" /> should be added for these domains?

[ivysrono]

@mathiasbynens Yes, you should do it.

1. Each target should be redirected by one rule. Otherwise, checks failed.
2. They only work work correctly over https. However, they have not be in preloadlist.

[mathiasbynens]

@ivysrono Thanks! Done.

[ivysrono]

Each target is one test url. No rule for http://forum.tibia.com/ and http://forum.test.tibia.com/ now.
This ruleset should be:

<ruleset name="tibia.com">
	<!--Complications:-->
	<target host="www.tibia.com" />
	<target host="www.test.tibia.com" />
	<target host="forum.tibia.com" />
	<target host="forum.test.tibia.com" />
	<rule from="^http://(www|forum)\.(test\.)?tibia\.com/" to="https://secure.test.tibia.com/" />

	<!--Directly:-->
	<target host="secure.tibia.com" />
	<target host="secure.test.tibia.com" />

	<rule from="^http:" to="https:" />
</ruleset>

[mathiasbynens]

<rule from="^http://(www|forum)\.(test\.)?tibia\.com/" to="https://secure.test.tibia.com/" />

This would redirect everything to secure.test.tibia.com, but I see what you mean. I’ve updated the file accordingly. Everything ok now?

[ivysrono]

@mathiasbynens Wait Collaborators...

[gloomy-ghost]

Thanks @mathiasbynens and @ivysrono. There are still some changes needed, please check out the list below. I'll merge this PR once the requested changes are approved.

• Add a rule that redirects http://forum.tibia.com/$ to https://secure.tibia.com/forum/?subtopic=worldboards, otherwise we would cause some confusing issues with the redirection.
• Same thing for http://forum.test.tibia.com/.
• A securecookie tag for ^secure\.(test\.)?tibia\.com$ would be nice.

[mathiasbynens]

@gloomy-ghost Done. PTAL

[gloomy-ghost]

Thanks, but the special case rule has to be at the position before a general rule because the general rule is applicable for targets that special rule is trying to redirect, and the special rule can't match anything if it's behind the general rule.

We prefer .+ over . in securecookie tags, they are functional equivalent though.

Also please don't squash commits when a reviewer has started requesting changes. We would like to use diff between commits rather than check everything again. PRs will be squash-and-merged in the end of reviewing anyway.

[gloomy-ghost]

Well, what I meant by "special case rule has to be at the position before a general rule" is switching these two lines and these two lines. securecookie is usually placed between targets and rules.

Also you can break a long rule into two lines. In this case it would be something like this:

<rule from="^http://(www|forum)\.test\.tibia\.com/"
	to="https://secure.test.tibia.com/" />

[mathiasbynens]

@gloomy-ghost Thanks for being so patient with me. I think I got it right this time!

[gloomy-ghost]

ERROR src/chrome/content/rules/tibia.com.xml: Not enough tests (1 vs 2) for <Rule from '^http://(www|forum)\.tibia\.com/' to 'https://secure.tibia.com/'>
ERROR src/chrome/content/rules/tibia.com.xml: Not enough tests (1 vs 2) for <Rule from '^http://(www|forum)\.test\.tibia\.com/' to 'https://secure.test.tibia.com/'>

Oops, it seems like the implicit test of target is used to pass the special rule, and now we don't have enough test for the general rule. I'd add a test for forum.tibia.com and a test for forum.test.tibia.com in order to pass the Travis CI.

[mathiasbynens]

Added. Tests seem to pass now!

[ivysrono]

format:

<ruleset name="tibia.com">
	<target host="www.tibia.com" />
	<target host="forum.tibia.com" />
	<rule from="^http://forum\.tibia\.com/$"
		to="https://secure.tibia.com/forum/" />
	<rule from="^http://(www|forum)\.tibia\.com/"
		to="https://secure.tibia.com/" />
		<test url="http://forum.tibia.com/forum/" />

	<target host="www.test.tibia.com" />
	<target host="forum.test.tibia.com" />
	<rule from="^http://forum\.test\.tibia\.com/$"
		to="https://secure.test.tibia.com/forum/" />
	<rule from="^http://(www|forum)\.test\.tibia\.com/"
		to="https://secure.test.tibia.com/" />
		<test url="http://forum.test.tibia.com/forum/" />

	<target host="secure.tibia.com" />
	<target host="secure.test.tibia.com" />

	<securecookie host="^secure\.(test\.)?tibia\.com$" name=".+" />

	<rule from="^http:" to="https:" />
</ruleset>

[gloomy-ghost]

The ruleset @ivysrono provided above represents the preferred style we are currently using except the indentation of a test that comes after a rule (the same position of to and test looks unnatural, I'll discuss this with other reviewers later on). In this PR the last thing I would suggest is adding newlines between securecookie and target rule then we are ready to go 😃

[mathiasbynens]

@gloomy-ghost Done.

[gloomy-ghost]

Thanks again!