Item9639: Merge CommentPlugin into release branch

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Enhancement	Needs Merge	Extension	CommentPlugin	CrawfordCurrie, PaulHarvey

Once 1.1 has gone, need to merge these CommentPlugin updates, which were blocked for 1.1, to the Release01x01 branch for 1.1.1

http://trac.foswiki.org/changeset/8955

-- CrawfordCurrie - 08 Sep 2010

This merges in the following reports: Item9568, Item9569, Item9591, Item9592, Item9601

These are all marked as Patch, and for 1.1.1

This bug report reminds us to sync over CommentPlugin to Release01x01 branch once 1.1.0 is released so it can go into first 1.1.1 patch release

-- KennethLavrsen - 13 Sep 2010

For sure not ready for 1.1.1

We need 1.1.1 before the remaining issues with this plugin are resolved. Especially redirect related problems with rest

-- KennethLavrsen - 19 Oct 2010

Crawford, Did any of this get merged into 1.1.x? The commit against this task is a reversal of a copy/paste error. The task was left set as ReleasedIn 1.1.1. Bumped to 1.1.4

-- GeorgeClark - 12 Mar 2011

No, it was not merged.

-- CrawfordCurrie - 13 Mar 2011

The rest handler needs more work; the try block for save is inside another try block which is problematic; and should only respond to POST requests.

-- PaulHarvey - 16 Mar 2011

But what you committed broke the ACL unit tests... Are you sure that's really what you want to do?

-- OlivierRaginel - 10 Jun 2011

I'm sure WikiGuest spam should cause a failing test too. I guess Crawford doesn't have time to fix it.

-- PaulHarvey - 10 Jun 2011

I explained on IRC why wikiguest is allowed to comment, unless they are explicitly excluded (if the rest script is not in AuthScripts? , then no auth is required to access it. That means it is up to local access controls to determine if wikiguest is able to comment or not. This is required because otherwise you have to be logged in to comment - and it's not always the case that you need to be.

-- CrawfordCurrie - 12 Jun 2011

I think it's a mistake that {AuthScripts} contradict the ACLs. Anyway, let's continue that discussion at SecurityChecklists.

On this task, TODO:

• Fix MANIFEST (missing comment.js)
• Restrict HTTP verb to POST only
• Require validation

-- PaulHarvey - 12 Jun 2011

Is this ready to merge, or do we defer to 1.1.5 if we are going to build 1.1.4?

-- GeorgeClark - 02 Oct 2011

No. Still needs validation & verb restriction in the registerRESTHandler, with corresponding js changes

-- PaulHarvey - 03 Oct 2011

Defer to 1.1.5

-- GeorgeClark - 13 Dec 2011

The nested try/catch issue seemed to be triggered by tainted values, and also incorrect syntax in the throw. Fixed under Item11443. It seems as the validation hook is the last thing missing before release.

-- GeorgeClark - 15 Jan 2012