refs #4671 implemented get_safe_url()

CMakeLists.txt

@@ -701,7 +701,7 @@ if (APPLE)
     set_target_properties(libqore PROPERTIES SOVERSION 12)
     set_target_properties(libqore PROPERTIES INSTALL_NAME_DIR ${CMAKE_INSTALL_FULL_LIBDIR})
 else (APPLE)
-    set_target_properties(libqore PROPERTIES VERSION 12.1.2)
+    set_target_properties(libqore PROPERTIES VERSION 12.2.0)
     set_target_properties(libqore PROPERTIES SOVERSION 12)
 endif (APPLE)
 

doxygen/lang/900_release_notes.dox.tmpl

@@ -26,6 +26,8 @@
       - added a \c path_params element to the \c UriQueryInfo typed hash to allow for reporting path arguments in
         calls supported by REST schemas
         (<a href="https://github.com/qorelanguage/qore/issues/4661">issue 4661</a>)
+    - added the @ref Qore::get_safe_url() "get_safe_url()" function to avoid exposing passwords in URLs
+      (<a href="https://github.com/qorelanguage/qore/issues/4671">issue 4671</a>)
 
     @subsection qore_1_13_bug_fixes Bug Fixes in Qore
     - <a href="../../modules/RestHandler/html/index.html">RestHandler</a> module

examples/test/qlib/ConnectionProvider/ConnectionProviderModule.qtest

@@ -43,7 +43,7 @@ public class ConnectionProviderModuleTest inherits QUnit::Test {
             p.parse("hash sub get_hash() { return get_connection_hash(); }", "");
             any obj = p.callFunction("get_obj", "X");
             assertEq(Type::Object, obj.type());
-            assertEq("test://user@x", obj.getInfo().url);
+            assertEq("test://user:<masked>@x", obj.getInfo().url);
             assertEq(NOTHING, obj.getInfo().url_hash.password);
             assertEq("test://user:pass@x", obj.getInfo(True).url);
             assertEq("pass", obj.getInfo(True).url_hash.password);

examples/test/qore/functions/get_qore_option_list.qtest

@@ -0,0 +1,25 @@
+#!/usr/bin/env qore
+# -*- mode: qore; indent-tabs-mode: nil -*-
+
+%new-style
+%enable-all-warnings
+%require-types
+%strict-args
+
+%requires ../../../../qlib/QUnit.qm
+
+%exec-class GetQoreOptionListTest
+
+public class GetQoreOptionListTest inherits QUnit::Test {
+    constructor() : Test("GetQoreOptionListTest", "1.0") {
+        addTestCase("get_qore_option_list() test", \getQoreOptionListTest());
+
+        # Return for compatibility with test harness that checks return value.
+        set_return_value(main());
+    }
+
+    getQoreOptionListTest() {
+        list<auto> l = get_qore_option_list();
+        assertEq("list<hash<auto>>", l.fullType());
+    }
+}

examples/test/qore/functions/get_safe_url.qtest

@@ -0,0 +1,29 @@
+#!/usr/bin/env qore
+# -*- mode: qore; indent-tabs-mode: nil -*-
+
+%new-style
+%enable-all-warnings
+%require-types
+%strict-args
+
+%requires ../../../../qlib/QUnit.qm
+
+%exec-class getSafeUrlTest
+
+public class getSafeUrlTest inherits QUnit::Test {
+    constructor() : Test("get_safe_url test", "1.0") {
+        addTestCase("main test", \mainTest());
+
+        # Return for compatibility with test harnesses that check the return value
+        set_return_value(main());
+    }
+
+    mainTest() {
+        assertEq("https://user:<masked>@site:8001/path", get_safe_url("https://user:password@site:8001/path"));
+        assertEq("https://user:<masked>@site:8001", get_safe_url("https://user:password@site:8001"));
+
+        assertEq("user:<masked>@site:8001/path", get_safe_url("user:password@site:8001/path"));
+        assertEq("user:<masked>@:8001/path", get_safe_url("user:pass@:8001/path"));
+        assertEq(":<masked>@:8001/path", get_safe_url(":pass@:8001/path"));
+    }
+}

examples/test/qore/functions/parse_url.qtest

@@ -73,6 +73,23 @@ public class parseUrlTest inherits QUnit::Test {
             "port": 587,
         }, h);
 
+        h = parse_url("user:pass@site:1001/path");
+        assertEq({
+            "username": "user",
+            "password": "pass",
+            "host": "site",
+            "port": 1001,
+            "path": "/path",
+        }, h);
+
+        h = parse_url("user:pass@:1001/path");
+        assertEq({
+            "username": "user",
+            "password": "pass",
+            "port": 1001,
+            "path": "/path",
+        }, h);
+
         # "standard" test
         assertEq(("protocol": "http", "path": "/path", "username": "user", "password": "pass", "host": "host", "port": 80), parse_url("http://user:pass@host:80/path"));
 

include/qore/intern/QoreHttpClientObjectIntern.h

@@ -117,20 +117,32 @@ struct con_info {
         return 0;
     }
 
-    DLLLOCAL QoreStringNode* get_url(bool suppress_password = false) const {
+    DLLLOCAL QoreStringNode* get_url(bool mask_password = false) const {
         QoreStringNode *pstr = new QoreStringNode("http");
         if (ssl) {
             pstr->concat("s://");
         } else {
             pstr->concat("://");
         }
+        bool has_username_or_password = false;
         if (!username.empty()) {
-            if (suppress_password) {
-                pstr->sprintf("%s@", username.c_str());
+            pstr->concat(username);
+            has_username_or_password = true;
+        }
+        if (!password.empty()) {
+            pstr->concat(':');
+            if (mask_password) {
+                pstr->concat("<masked>");
             } else {
-                pstr->sprintf("%s:%s@", username.c_str(), password.c_str());
+                pstr->concat(password);
+            }
+            if (!has_username_or_password) {
+                has_username_or_password = true;
             }
         }
+        if (has_username_or_password) {
+            pstr->concat('@');
+        }
 
         if (!port) {
             // concat and encode "host" when using a UNIX domain socket

lib/Makefile.am

@@ -11,7 +11,7 @@ dummy:
 	echo "Build started!"
 
 EXTRA_INCLUDES = -I$(top_srcdir)/include -I$(top_builddir)/include -I$(top_builddir)/lib
-libqore_la_LDFLAGS = -version-info 13:2:1 -no-undefined ${QORE_LIB_LDFLAGS}
+libqore_la_LDFLAGS = -version-info 14:0:2 -no-undefined ${QORE_LIB_LDFLAGS}
 AM_CPPFLAGS = $(EXTRA_INCLUDES) ${QORE_LIB_CPPFLAGS}
 AM_CXXFLAGS = ${QORE_LIB_CXXFLAGS}
 AM_YFLAGS = -d

lib/ql_misc.qpp

@@ -1167,6 +1167,67 @@ hash<UrlInfo> parse_url(string url, *int options) {
     return qurl.isValid() ? qurl.getHash() : QoreValue();
 }
 
+//! Returns the URL string passed without any password information
+/** @param url the URL to process
+
+    @return the URL string passed without any password information
+
+    @since %Qore 1.13
+*/
+string get_safe_url(string url) {
+    QoreURL qurl(xsink, url, QURL_KEEP_BRACKETS);
+    if (!qurl.isValid()) {
+        assert(*xsink);
+        return QoreValue();
+    }
+    if (!qurl.getPassword()) {
+        url->ref();
+        return url;
+    }
+
+    SimpleRefHolder<QoreStringNode> rv(new QoreStringNode(QCS_UTF8));
+
+    const QoreString* str = qurl.getProtocol();
+    if (str && !str->empty()) {
+        rv->concat(str, xsink);
+        rv->concat("://");
+    }
+
+    str = qurl.getUserName();
+    bool has_user_or_pass = false;
+    if (str && !str->empty()) {
+        rv->concat(str, xsink);
+        has_user_or_pass = true;
+    }
+
+    str = qurl.getPassword();
+    if (str && !str->empty()) {
+        rv->concat(":<masked>");
+        if (!has_user_or_pass) {
+            has_user_or_pass = true;
+        }
+    }
+    if (has_user_or_pass) {
+        rv->concat('@');
+    }
+
+    str = qurl.getHost();
+    if (str && !str->empty()) {
+        rv->concat(str, xsink);
+    }
+
+    if (qurl.getPort()) {
+        rv->sprintf(":%d", qurl.getPort());
+    }
+
+    str = qurl.getPath();
+    if (str && !str->empty()) {
+        rv->concat(str, xsink);
+    }
+
+    return rv.release();
+}
+
 //! This function variant does nothing at all; it is only included for backwards-compatibility with qore prior to version 0.8.0 for functions that would ignore type errors in arguments
 /**
  */

qlib/AwsRestClient.qm

@@ -228,7 +228,8 @@ AwsRestClient rest({
         # AWS does not support basic authentication
         hash<UrlInfo> url_info = parse_url(opts.url);
         if (url_info{"username", "password"}) {
-            throw "AWSRESTCLIENT-ERROR", sprintf("AWS URL must not contain a username or password; URL: %y", opts.url);
+            throw "AWSRESTCLIENT-ERROR", sprintf("AWS URL must not contain a username or password; URL: %y",
+                get_safe_url(opts.url));
         }
 
         # set the region automatically from the URL if not provided in an option
@@ -269,7 +270,8 @@ AwsRestClient rest({
         }
     }
 
-    hash<auto> sendAndDecodeResponse(*data body, string m, string path, hash<auto> hdr, *reference<hash<auto>> info, *softbool decode_errors) {
+    hash<auto> sendAndDecodeResponse(*data body, string m, string path, hash<auto> hdr, *reference<hash<auto>> info,
+            *softbool decode_errors) {
         # get request time
         date gmtime = Qore::gmtime();
         # get credential scope
@@ -311,8 +313,8 @@ AwsRestClient rest({
         return sig;
     }
 
-    private string getRequestString(string http_method, string path, reference<hash<auto>> hdr, *data body, date gmtime,
-        string scope, reference<string> signed_headers) {
+    private string getRequestString(string http_method, string path, reference<hash<auto>> hdr, *data body,
+            date gmtime, string scope, reference<string> signed_headers) {
         # get AWS date value
         string aws_date = gmtime.format("YYYYMMDDTHHmmSS") + "Z";
         hdr."X-Amz-Date" = aws_date;
@@ -359,7 +361,8 @@ AwsRestClient rest({
             cstr += (
                 foldl $1 + "&" + $2, (
                     map
-                        sprintf("%s=%s", encode_url($1, True), uri_info.params{$1} === True ? "" : encode_url(uri_info.params{$1}, True)),
+                        sprintf("%s=%s", encode_url($1, True),
+                            uri_info.params{$1} === True ? "" : encode_url(uri_info.params{$1}, True)),
                         sort(keys uri_info.params)
                 )
             );

qlib/CdsRestDataProvider/CdsRestDataProvider.qc

@@ -161,7 +161,7 @@ public class CdsRestDataProvider inherits DataProvider::AbstractDataProvider {
         } catch (hash<ExceptionInfo> ex) {
             # ensure that any error response body is included in the exception
             hash<auto> ex_arg = info{"request-uri", "response-code", "response-body"};
-            throw ex.err, ex.desc, ex.arg + ex_arg;
+            rethrow ex.err, ex.desc, ex.arg + ex_arg;
         }
         #printf("%N\n", resp.EntityType);
 

qlib/ConnectionProvider/AbstractConnection.qc

@@ -469,8 +469,20 @@ scheme://user:pass@hostname:port/path
     */
     private string getSafeUrl(hash<auto> urlh) {
         string url = urlh.protocol + "://";
-        if (urlh.username)
-            url += urlh.username + "@";
+        bool has_user_or_pass;
+        if (urlh.username) {
+            url += urlh.username;
+            has_user_or_pass = True;
+        }
+        if (urlh.password) {
+            url += ":<masked>";
+            if (!has_user_or_pass) {
+                has_user_or_pass = True;
+            }
+        }
+        if (has_user_or_pass) {
+            url += "@";
+        }
         url += urlh.host;
         if (urlh.port)
             url += ":" + urlh.port;

qlib/Pop3Client.qm

@@ -257,7 +257,8 @@ Pop3Client pop3("pop3s://user@gmail.com:password@pop.gmail.com");
         else {
             *hash conf = Protocols.(hurl.protocol.lwr());
             if (!exists conf) {
-                throw "POP3-URL-ERROR", sprintf("unknown scheme %y in %y; known schemes: %y", hurl.protocol, url, keys Protocols);
+                throw "POP3-URL-ERROR", sprintf("unknown scheme %y in %y; known schemes: %y", hurl.protocol,
+                    get_safe_url(url), keys Protocols);
             }
             tls = conf.tls;
             if (!hurl.port && hurl.host[0] != "/") {
@@ -266,7 +267,7 @@ Pop3Client pop3("pop3s://user@gmail.com:password@pop.gmail.com");
         }
 
         if (!hurl.host.val())
-            throw "POP3-URL-ERROR", sprintf("no hostname was given in URL %y", url);
+            throw "POP3-URL-ERROR", sprintf("no hostname was given in URL %y", get_safe_url(url));
 
         # if the hostname is an integer, then assume the given port on localhost
         if (hurl.host.val()) {
@@ -285,7 +286,7 @@ Pop3Client pop3("pop3s://user@gmail.com:password@pop.gmail.com");
         }
 
         if (!exists hurl.username) {
-            throw "POP3-URL-ERROR", sprintf("missing username in POP3 URL: %y", url);
+            throw "POP3-URL-ERROR", sprintf("missing username in POP3 URL: %y", get_safe_url(url));
         }
 
         if (!exists hurl.password) {
@@ -1043,7 +1044,7 @@ public class Pop3Connection inherits ConnectionProvider::AbstractConnectionWithI
         @throw CONNECTION-OPTION-ERROR missing or invalid connection option
     */
     constructor(string name, string description, string url, hash attributes = {}, hash options = {})
-        : AbstractConnectionWithInfo(name, description, url, attributes, options) {
+            : AbstractConnectionWithInfo(name, description, url, attributes, options) {
     }
 
     #! returns \c "pop3"

qlib/RestClient.qm

@@ -1043,6 +1043,8 @@ hash<auto> ans = rest.doRequest("DELETE", "/orders/1");
         # prepare path
         preparePath(\path);
 
+        on_error rethrow $1.err, sprintf("%s (REST URL %y)", $1.desc, getSafeURL());
+
         return sendAndDecodeResponse(body, m, path, hdr, \info, decode_errors);
     }
 

qlib/Swagger.qm

@@ -896,6 +896,9 @@ public class SwaggerLoader {
                 json = True;
             }
         }
+        on_error {
+            rethrow $1.err, sprintf("%s (from URL %y)", $1.desc, get_safe_url(url));
+        }
         return SwaggerLoader::fromString(FileLocationHandler::getTextFileFromLocation(url), json);
     }
 
@@ -930,7 +933,7 @@ public class SwaggerLoader {
 
     #! parses the source encoding from the given string
     static hash<auto> parseSchemaSource(string str, string ser) {
-        hash<auto> rv;
+        auto rv;
         switch (ser) {
             case "json": {
 %ifdef NoJson
@@ -957,6 +960,10 @@ public class SwaggerLoader {
 %endif
             }
         }
+        if (rv.typeCode() != NT_HASH) {
+            throw "SWAGGER-SCHEMA-ERROR", sprintf("data of type %y provided for OpenAPI / Swagger schema; expecting "
+                "\"string\"", rv.fullType());
+        }
         return rv;
     }