Skip to content
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

JOINDIN-475 Use Forwarded and X-Forwarded-For headers in Akismet check #122

Merged
merged 7 commits into from
Dec 15, 2014

Conversation

adear11
Copy link

@adear11 adear11 commented Oct 27, 2014

JOINDIN-475

Added a new Header class that represents a Header and all of its values. This class is used by the Request class to properly set the source IP by looking for either a Forwarded header or a X-Forwarded-For header. If the Forwarded header is present, it also attempts to set the User-Agent from the values it contains.

adear added 7 commits September 8, 2014 00:57
Add new method to Request object to set the user's IP address based on whether or not there is a X-Forwarded-For is present.

Also, refactor the SpamCheckService to have the ip address and user agent passed in rather than pulling them directly from _SERVER. This included updating the two usages of SpamCheckService::isCommentAcceptable to pass in the ip address and user agent.
…der class, and update Request class to use this new class.
Wasn't paying attention to what I was doing when I fixed the previous merge conflict, and removed this line.
@joindin-jenkins
Copy link

Can one of the admins verify this patch?

@akrabat
Copy link
Contributor

akrabat commented Oct 27, 2014

ok to test

@akrabat
Copy link
Contributor

akrabat commented Oct 29, 2014

this is ok to test

* @return Boolean true if the comment is okay, false if it got rated as spam
*/
public function isCommentAcceptable($data) {
public function isCommentAcceptable($data,$userIp,$userAgent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we not be better off accepting a Request object here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I don't like services being aware of the outside world as represented by Request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's about an interface representing a commenting user? That would be more extensible in the future and so this method signature wouldn't have to change if we suddenly need to judge whether something's spam on another service

@akrabat akrabat merged commit bc05b02 into joindin:master Dec 15, 2014
@akrabat
Copy link
Contributor

akrabat commented Dec 15, 2014

Thanks!

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.

4 participants