Accessing a public group pad without session no longer causes a crash

[luniki]

I created a group pad via HTTP API and set the public status to true:

https://studip-etherpad.uni-osnabrueck.de:9001/p/g.miq3NkqS2mCkMCnj%24shouldbepublic

If I access this URI in another browser (without a session), etherpad logs an error:

[2019-04-18 11:09:03.172] [ERROR] console - (node:115680) UnhandledPromiseRejectionWarning: apierror: sessionID does not exist at Object.exports.getSessionInfo (/opt/etherpad-lite/src/node/db/SessionManager.js:128:11) at at process._tickDomainCallback (internal/process/next_tick.js:229:7)

My guess is that the changes in https://github.com/ether/etherpad-lite/commit/769933786ceabf6aac87f788fef5560270e4db93#diff-388a39eb587bc89d31d7d2d4036e51fcL99 are the cause for this error. The call to "sessionManager.getSessionInfo(sessionID)" should be in the try/catch block not outside of it.

[seyuf]

Hi @raybellis ,

Can you fix this please? The solution has been mentioned by @luniki above. You just have to make the try catch englobe the Promise.all and the for loop. This is kinda critical and seems quite easy to fix... Also seems to have many related issues e.g #3642

Regards

[muxator]

I've put together a bash script to perform the necessary API calls on https://gist.github.com/muxator/b7913aa2957286ad2f8cf9a303dec6ef

Requirements:

• a running etherpad on http://localhost:9001
• jq must be installed

Example of its output:

$ ./etherpad-issue-3600.sh 
1. create a group: GROUP_ID=g.Ftlk4BoRfyzcISWs
2. create a group pad inside that group: PAD_ID=g.Ftlk4BoRfyzcISWs$shouldbepublic
3. set the public status of that pad to true: ok
4. now visit http://localhost:9001/p/g.Ftlk4BoRfyzcISWs$shouldbepublic

I can confirm that with Etherpad 1.7.5 the pad is visible, with etherpad 1.8.0 there is an error.

[muxator]

@seyuf: thanks for bumping this up. I am not sure, however, that #3642 is related to this.

The problem here is in the code of SecurityManager.js. #3642 is probably due to another cause. If you still see a commonality, please share your thoughts there, so we can keep this issue focused on a single topic.

Thanks :)

[muxator]

I thought it would be a good idea to write a test for catching this case, so I tried updating tests/backend/specs/api/sessionsAndGroups.js to try to read via API the text of a public group pad.

diff --git a/tests/backend/specs/api/sessionsAndGroups.js b/tests/backend/specs/api/sessionsAndGroups.js
--- a/tests/backend/specs/api/sessionsAndGroups.js
+++ b/tests/backend/specs/api/sessionsAndGroups.js
@@ -298,6 +298,18 @@ describe('getPublicStatus', function(){
   });
 })

+describe('getText', function(){
+  it('Reads the text of a public group pad', function(done) {
+    api.get(endPoint('getText')+"&padID="+padID)
+    .expect(function(res){
+      console.log("Let's just fail and show the response");
+      throw new Error(JSON.stringify(res.body));
+    })
+    .expect('Content-Type', /json/)
+    .expect(200, done)
+  });
+})
+
 describe('isPasswordProtected', function(){
   it('Gets the public status of a pad', function(done) {
     api.get(endPoint('isPasswordProtected')+"&padID="+padID)

I was hoping that test would break, generating the same error described in this issue.

It did not, and correctly returned the pad contents.

It turns out that the getText() function in the API returns the pad contents independently of the public status of the pad, differently from the GUI...

[muxator]

Fixed.

There is currently no way of catching this kind of bugs (that only manifest when visiting via the browser) unsing only the backend tests. See previous comment.

Will be released in 1.8.3.