You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ponymail.apache.org by se...@apache.org on 2016/12/19 20:39:01 UTC

incubator-ponymail git commit: pcall() idiom to protect against elastic.lua exceptions is flawed

Repository: incubator-ponymail
Updated Branches:
  refs/heads/master b8ed9186e -> 5d09ac1c6


pcall() idiom to protect against elastic.lua exceptions is flawed

This fixes #162

Project: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/commit/5d09ac1c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/tree/5d09ac1c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/diff/5d09ac1c

Branch: refs/heads/master
Commit: 5d09ac1c62990c884418f1aa3d181a33fbcf42af
Parents: b8ed918
Author: Sebb <se...@apache.org>
Authored: Mon Dec 19 20:38:39 2016 +0000
Committer: Sebb <se...@apache.org>
Committed: Mon Dec 19 20:38:39 2016 +0000

----------------------------------------------------------------------
 CHANGELOG.md             |  1 +
 site/api/email.lua       |  2 +-
 site/api/lib/elastic.lua | 20 +++++++++++---------
 site/api/lib/user.lua    |  2 +-
 site/api/source.lua      |  2 +-
 site/api/thread.lua      |  2 +-
 6 files changed, 16 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/CHANGELOG.md
----------------------------------------------------------------------
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2891379..652eec4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -89,6 +89,7 @@
 - preferences does not properly remove nulls from account.credentials.altemail (#309)
 - manage e-mails can create multiple identical alternate addresses (#307)
 - elastic.get does not return if a document is not found but some callers overlook this (#137)
+- pcall() idiom to protect against elastic.lua exceptions is flawed (#162)
 
 ## CHANGES in 0.9b:
 

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/site/api/email.lua
----------------------------------------------------------------------
diff --git a/site/api/email.lua b/site/api/email.lua
index 825a4f1..8aea651 100644
--- a/site/api/email.lua
+++ b/site/api/email.lua
@@ -29,7 +29,7 @@ function handle(r)
     cross.contentType(r, "application/json")
     local get = r:parseargs()
     local eid = (get.id or ""):gsub("\"", "")
-    local _, doc = pcall(function() return elastic.get("mbox", eid or "hmm") end)
+    local doc = elastic.get("mbox", eid or "hmm", true)
     
     -- Try searching by original source mid if not found, for backward compat
     if not doc or not doc.mid then

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/site/api/lib/elastic.lua
----------------------------------------------------------------------
diff --git a/site/api/lib/elastic.lua b/site/api/lib/elastic.lua
index e5bdd67..560cf85 100644
--- a/site/api/lib/elastic.lua
+++ b/site/api/lib/elastic.lua
@@ -26,11 +26,11 @@ local default_doc = "mbox"
 -- N.B. if the index is closed, ES returns 403, but that may perhaps be true for other conditions
 -- ES returns 404 if the index is missing
 -- ES also returns 404 if a document is missing
-local function checkReturn(code)
+local function checkReturn(code, ok404)
     if type(code) == "number" then -- we have a valid HTTP status code
         -- ignore expected return codes here
         -- index returns 201 when an entry is created
-        if code ~= 200 and code ~= 201 then
+        if code ~= 200 and code ~= 201 and not (ok404 and code == 404) then
             -- code is called by 2nd-level functions only, so level 4 is the external caller
             error("Backend Database returned code " .. code .. "!", 4)
         end
@@ -48,20 +48,22 @@ end
 -- Parameters:
 --  - url (required)
 --  - query (optional); if this is a table it is decoded into JSON
+--  - ok404 (optional); if true, then 404 is allowed as a status return
 -- returns decoded JSON result
 -- may throw an error if the request fails
---
-local function performRequest(url, query) 
+-- Returns:
+-- json, status code (i.e. 200,201 or 404)
+local function performRequest(url, query, ok404) 
     local js = query
     if type(query) == "table" then
         js = JSON.encode(query)
     end
     local result, hc = http.request(url, js)
-    checkReturn(hc)
+    checkReturn(hc, ok404)
     local json = JSON.decode(result)
     -- TODO should we return the http status code?
     -- This might be necessary if codes such as 404 did not cause an error
-    return json
+    return json, hc
 end
 
 -- Standard ES query, returns $size results of any doc of type $doc, sorting by $sitem
@@ -87,16 +89,16 @@ local function getHits(query, size, doc, sitem)
 end
 
 -- Get a single document
-local function getDoc (ty, id)
+local function getDoc (ty, id, ok404)
     local url = config.es_url  .. ty .. "/" .. id
-    local json = performRequest(url)
+    local json, status = performRequest(url, nil, ok404)
     if json and json._source then
         json._source.request_id = json._id
         if ty == "mbox" and json._source.body == JSON.null then
             json._source.body = ''
         end
     end
-    return (json and json._source) and json._source or {}
+    return (json and json._source) and json._source or {}, status
 end
 
 -- Get results (a'la getHits), but only return email headers, not the body

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/site/api/lib/user.lua
----------------------------------------------------------------------
diff --git a/site/api/lib/user.lua b/site/api/lib/user.lua
index 712517a..2f2d4c2 100644
--- a/site/api/lib/user.lua
+++ b/site/api/lib/user.lua
@@ -24,7 +24,7 @@ local function getUser(r, override)
     if override or (ocookie and #ocookie > 43) then
         local cookie, cid = r:unescape(ocookie or ""):match("([a-f0-9]+)==(.+)")
         if override or (cookie and #cookie >= 40 and cid) then
-            local _, js = pcall(function() return elastic.get('account', r:sha1(override or cid)) end)
+            local js = elastic.get('account', r:sha1(override or cid), true)
             if js and js.credentials and (override or (cookie == js.internal.cookie)) then
                 login = {
                     credentials = {

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/site/api/source.lua
----------------------------------------------------------------------
diff --git a/site/api/source.lua b/site/api/source.lua
index afcee70..7332e56 100644
--- a/site/api/source.lua
+++ b/site/api/source.lua
@@ -27,7 +27,7 @@ function handle(r)
     cross.contentType(r, "text/plain")
     local get = r:parseargs()
     local eid = (get.id or r.path_info):gsub("\"", ""):gsub("/", "")
-    local _, doc = pcall(function() return elastic.get("mbox", eid or "hmm") end)
+    local doc = elastic.get("mbox", eid or "hmm", true)
     
     -- Try searching by mid if not found, for backward compat
     if not doc or not doc.mid then

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/5d09ac1c/site/api/thread.lua
----------------------------------------------------------------------
diff --git a/site/api/thread.lua b/site/api/thread.lua
index 97e8152..1ee94d5 100644
--- a/site/api/thread.lua
+++ b/site/api/thread.lua
@@ -73,7 +73,7 @@ function handle(r)
     local now = r:clock()
     local get = r:parseargs()
     local eid = (get.id or ""):gsub("\"", "")
-    local _, doc = pcall(function() return elastic.get("mbox", eid or "hmm") end)
+    local doc = elastic.get("mbox", eid or "hmm", true)
     emls_thrd = {}
     -- Try searching by mid if not found, for backward compat
     if not doc or not doc.mid then