11:08 a.m.
If we're contacting a webdav server, trying to retrieve properties at an
address like https://example.com/dav/username/events, a likely reason for
a 404 error is that the username is invalid. We can't have a proper
heuristic to determine that, so let's just assume an authentication
error and inform the user accordingly.
11:08 a.m.
New subject: [SyncEvolution] [PATCH 1/2] NeonCXX.{h, cpp}: rename check to check_error.
Using a name that gives better context.
---
src/backends/webdav/NeonCXX.cpp | 12 ++++++------
src/backends/webdav/NeonCXX.h | 16 ++++++++--------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backends/webdav/NeonCXX.cpp b/src/backends/webdav/NeonCXX.cpp
index eacc907..f40832b 100644
--- a/src/backends/webdav/NeonCXX.cpp
+++ b/src/backends/webdav/NeonCXX.cpp
@@ -354,7 +354,7 @@ int Session::sslVerify(void *userdata, int failures, const
ne_ssl_certificate *c
unsigned int Session::options(const std::string &path)
{
unsigned int caps;
- check(ne_options2(m_session, path.c_str(), &caps));
+ check_error(ne_options2(m_session, path.c_str(), &caps));
return caps;
}
#endif // HAVE_LIBNEON_OPTIONS
@@ -394,7 +394,7 @@ void Session::propfindURI(const std::string &path, int depth,
StringPrintf("%d status: redirected to %s", code,
location.c_str()),
code, location);
} else {
- if (!check(error, code)) {
+ if (!check_error(error, code)) {
goto retry;
}
}
@@ -470,7 +470,7 @@ void Session::flush()
}
}
-bool Session::check(int error, int code, const ne_status *status, const string
&location)
+bool Session::check_error(int error, int code, const ne_status *status, const string
&location)
{
flush();
@@ -810,7 +810,7 @@ bool Request::run()
error = ne_xml_dispatch_request(m_req, m_parser->get());
}
- return check(error);
+ return check_error(error);
}
int Request::addResultData(void *userdata, const char *buf, size_t len)
@@ -820,9 +820,9 @@ int Request::addResultData(void *userdata, const char *buf, size_t
len)
return 0;
}
-bool Request::check(int error)
+bool Request::check_error(int error)
{
- return m_session.check(error, getStatus()->code, getStatus(),
getResponseHeader("Location"));
+ return m_session.check_error(error, getStatus()->code, getStatus(),
getResponseHeader("Location"));
}
}
diff --git a/src/backends/webdav/NeonCXX.h b/src/backends/webdav/NeonCXX.h
index f4b5407..6fde70c 100644
--- a/src/backends/webdav/NeonCXX.h
+++ b/src/backends/webdav/NeonCXX.h
@@ -277,7 +277,7 @@ class Session {
*
* call sequence is this:
* - startOperation()
- * - repeat until success or final failure: create request, run(), check()
+ * - repeat until success or final failure: create request, run(), check_error()
*
* @param operation internal descriptor for debugging (for example, PROPFIND)
* @param deadline time at which the operation must be completed, otherwise
it'll be considered failed;
@@ -287,7 +287,7 @@ class Session {
/**
* to be called after each operation which might have produced debugging output by
neon;
- * automatically called by check()
+ * automatically called by check_error()
*/
void flush();
@@ -303,7 +303,7 @@ class Session {
* @return true for success, false if retry needed (only if deadline not empty);
* errors reported via exceptions
*/
- bool check(int error, int code = 0, const ne_status *status = NULL,
+ bool check_error(int error, int code = 0, const ne_status *status = NULL,
const string &location = "");
ne_session *getSession() const { return m_session; }
@@ -321,9 +321,9 @@ class Session {
ne_session *m_session;
URI m_uri;
std::string m_proxyURL;
- /** time when last successul request completed, maintained by check() */
+ /** time when last successul request completed, maintained by check_error() */
Timespec m_lastRequestEnd;
- /** number of times a request was sent, maintained by startOperation(), the
credentials callback, and check() */
+ /** number of times a request was sent, maintained by startOperation(), the
credentials callback, and check_error() */
int m_attempt;
/** ne_set_server_auth() callback */
@@ -485,10 +485,10 @@ class Request
/**
* Execute the request. May only be called once per request. Uses
- * Session::check() underneath to detect fatal errors and throw
+ * Session::check_error() underneath to detect fatal errors and throw
* exceptions.
*
- * @return result of Session::check()
+ * @return result of Session::check_error()
*/
bool run();
@@ -514,7 +514,7 @@ class Request
static int addResultData(void *userdata, const char *buf, size_t len);
/** throw error if error code *or* current status indicates failure */
- bool check(int error);
+ bool check_error(int error);
};
/** thrown for 301 HTTP status */
--
1.7.4.1
11:08 a.m.
New subject: [SyncEvolution] [PATCH 2/2] WebDavSource.cpp: hijack error 404 to 401 when appropriate.
When doing the first propfindProp, if we get a 404, it might mean
that the username was wrong, so the server gave us a not found
error. It's better to let the user know that, because we don't
have a clear heuristic to determin whether this might have been
a true 404 error.
See also: https://bugs.meego.com/show_bug.cgi?id=17862
---
src/backends/webdav/WebDAVSource.cpp | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/src/backends/webdav/WebDAVSource.cpp b/src/backends/webdav/WebDAVSource.cpp
index 816eba8..b409b71 100644
--- a/src/backends/webdav/WebDAVSource.cpp
+++ b/src/backends/webdav/WebDAVSource.cpp
@@ -551,6 +551,17 @@ void WebDAVSource::contactServer()
} else {
candidates.push_front(next.m_path);
}
+ } catch (const TransportStatusException &ex) {
+ SE_LOG_DEBUG(NULL, NULL, "TransportStatusException: %s",
ex.what());
+ if (ex.syncMLStatus() == 404) {
+ // We're actually looking at an authentication error: the path to the
calendar has
+ // not been found, so the username was wrong. Let's hijack the error
message and
+ // code of the exception by throwing a new one.
+ string descr = StringPrintf("Path not found: %s. Is the username
'%s' correct?",
+ path.c_str(), username.c_str());
+ int code = 401;
+ SE_THROW_EXCEPTION_STATUS(TransportStatusException, descr,
SyncMLStatus(code));
+ }
} catch (const Exception &ex) {
if (candidates.empty()) {
// nothing left to try, bail out with this error
--
1.7.4.1
3:40 p.m.
On Mo, 2011-05-23 at 11:08 +0100, Salvatore Iovene wrote:
If we're contacting a webdav server, trying to retrieve
properties at an
address like https://example.com/dav/username/events, a likely reason for
a 404 error is that the username is invalid. We can't have a proper
heuristic to determine that, so let's just assume an authentication
error and inform the user accordingly.
Not merged. See my comments from the merge request:
check_error(): not in line with function naming, use checkError()
Translating the 404 error into 401 is too aggressive. It should only be
done if the path for which the error was triggered really was the one
where %u was substituted.
Please rework these patches and commit as a new branch to the upstream
repo.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3879
days inactive
3894
days old
syncevolution@synevolution.org
3 comments
2 participants
participants (2)
-
Patrick Ohly -
Salvatore Iovene