diff options
| -rw-r--r-- | mod/jwt.cxx | 19 | ||||
| -rw-r--r-- | mod/jwt.hxx | 2 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 74 | ||||
| -rw-r--r-- | mod/module.cli | 6 |
4 files changed, 49 insertions, 52 deletions
diff --git a/mod/jwt.cxx b/mod/jwt.cxx index 0c9c1f6..6176f27 100644 --- a/mod/jwt.cxx +++ b/mod/jwt.cxx @@ -55,7 +55,7 @@ string brep:: gen_jwt (const options::openssl_options& o, const path& pk, const string& iss, - const chrono::minutes& vp, + const chrono::seconds& vp, const chrono::seconds& bd) { // Create the header. @@ -125,12 +125,11 @@ gen_jwt (const options::openssl_options& o, ifdstream err (move (errp.in)); vector<char> bs; // Binary signature (openssl output). - string et; // Openssl stderr text. try { // In case of exception, skip and close input after output. // - // Note: re-open in/out so that they get automaitcally closed on + // Note: re-open in/out so that they get automatically closed on // exception. // ifdstream in (os.in.release (), fdstream_mode::skip); @@ -155,16 +154,16 @@ gen_jwt (const options::openssl_options& o, { throw_generic_error ( e.code ().value (), - ("unable to read/write openssl stdout/stdin: " + e.what ()).c_str ()); + (string ("unable to read/write openssl stdout/stdin: ") + + e.what ()).c_str ()); } } if (!os.wait ()) { - et = err.read_text (); - throw_generic_error ( - EINVAL, - ("non-zero openssl exit status: " + et).c_str ()); + string et (err.read_text ()); + throw_generic_error (EINVAL, + ("non-zero openssl exit status: " + et).c_str ()); } err.close (); @@ -175,7 +174,7 @@ gen_jwt (const options::openssl_options& o, { throw_generic_error ( e.code ().value (), - ("unable to execute openssl: " + e.what ()).c_str ()); + (string ("unable to execute openssl: ") + e.what ()).c_str ()); } catch (const io_error& e) { @@ -183,7 +182,7 @@ gen_jwt (const options::openssl_options& o, // throw_generic_error ( e.code ().value (), - ("unable to read openssl stderr : " + e.what ()).c_str ()); + (string ("unable to read openssl stderr : ") + e.what ()).c_str ()); } return h + '.' + p + '.' + s; // Return the token. diff --git a/mod/jwt.hxx b/mod/jwt.hxx index 4b5a54f..550649f 100644 --- a/mod/jwt.hxx +++ b/mod/jwt.hxx @@ -33,7 +33,7 @@ namespace brep gen_jwt (const options::openssl_options&, const path& private_key, const string& issuer, - const std::chrono::minutes& validity_period, + const std::chrono::seconds& validity_period, const std::chrono::seconds& backdate = std::chrono::seconds (60)); } diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 8705b17..bd68452 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -216,17 +216,19 @@ github_post (T& rs, const string& ep, const brep::strings& hdrs) // stdin because it will cause butl::curl to fail if the method is // POST. // - // @@ TODO: let's redirect stderr like in JWT. - // + fdpipe errp (fdopen_pipe ()); // stderr pipe. + curl c (path ("-"), path ("-"), // Write response to curl::in. - 2, + process::pipe (errp.in.get (), move (errp.out)), curl::post, "https://api.github.com/" + ep, "--write-out", "{\"brep_http_status\": %{http_code}}\n", "--header", "Accept: application/vnd.github+json", "--header", "X-GitHub-Api-Version: 2022-11-28", move (hdr_opts)); + ifdstream err (move (errp.in)); + // Parse the HTTP response. // int sc; // Status code. @@ -268,40 +270,39 @@ github_post (T& rs, const string& ep, const brep::strings& hdrs) } catch (const brep::io_error& e) { - // IO failure, child exit status doesn't matter. Just wait for the - // process completion and throw. + // If the process exits with non-zero status, assume the IO error is due + // to that and fall through. // - c.wait (); - - throw_generic_error ( + if (c.wait ()) + { + throw_generic_error ( e.code ().value (), - (string ("unable to communicate with curl: ") + e.what ()) - .c_str ()); + (string ("unable to read curl stdout: ") + e.what ()).c_str ()); + } } catch (const json::invalid_json_input& e) { - if (!c.wait ()) + // If the process exits with non-zero status, assume the JSON error is + // due to that and fall through. + // + if (c.wait ()) { throw_generic_error ( - EINVAL, - ("curl failed with " + to_string (*c.exit)).c_str ()); - } - - throw runtime_error ( + EINVAL, (string ("malformed JSON response from GitHub: ") + e.what ()) - .c_str ()); + .c_str ()); + } } - // @@ TMP The odds of this failing are probably slim given that we parsed - // the JSON output successfully. - // if (!c.wait ()) { - throw_generic_error ( - EINVAL, - ("curl failed with " + to_string (*c.exit)).c_str ()); + string et (err.read_text ()); + throw_generic_error (EINVAL, + ("non-zero curl exit status: " + et).c_str ()); } + err.close (); + return sc; } catch (const process_error& e) @@ -310,6 +311,14 @@ github_post (T& rs, const string& ep, const brep::strings& hdrs) e.code ().value (), (string ("unable to execute curl:") + e.what ()).c_str ()); } + catch (const brep::io_error& e) + { + // Unable to read diagnostics from stderr. + // + throw_generic_error ( + e.code ().value (), + (string ("unable to read curl stderr : ") + e.what ()).c_str ()); + } } bool brep::ci_github:: @@ -386,16 +395,6 @@ handle (request& rq, response& rs) } catch (const json::invalid_json_input& e) { - // @@ TODO: should we write more detailed diagnostics to log? Maybe we - // should do this for all unsuccessful calls to respond(). - // - // Note: these exceptions end up in the apache error log. - // - // @@ TMP Actually I was wrong, these do not end up in any logs. Pretty - // sure I saw them go there but they're definitely not anymore. - // - // See how it's done in other modules and do the same. - // throw invalid_request (400, "malformed JSON in request body"); } @@ -430,7 +429,7 @@ handle (request& rq, response& rs) *options_, options_->ci_github_app_private_key (), to_string (options_->ci_github_app_id ()), - chrono::minutes (options_->ci_github_jwt_validity_period ()), + chrono::seconds (options_->ci_github_jwt_validity_period ()), chrono::seconds (60)); cout << "JWT: " << jwt << endl; @@ -471,11 +470,11 @@ handle (request& rq, response& rs) } catch (const system_error& e) { - fail << "unable to get installation access token: [" << e.code () - << "] " << e.what (); + fail << "unable to get installation access token (errno=" << e.code () + << "): " << e.what (); } - cout << "<installation_access_token>" << endl << iat << endl; + cout << endl << "<installation_access_token>" << endl << iat << endl; return true; } @@ -607,7 +606,6 @@ operator<< (ostream& os, const check_suite_event& cs) os << "<check_suite>" << endl << cs.check_suite; os << "<repository>" << endl << cs.repository; os << "<installation>" << endl << cs.installation; - os << endl; return os; } diff --git a/mod/module.cli b/mod/module.cli index 9b17a2c..a829659 100644 --- a/mod/module.cli +++ b/mod/module.cli @@ -869,10 +869,10 @@ namespace brep the GitHub app's settings." } - uint16_t ci-github-jwt-validity-period = 10 // @@ Seconds. + uint16_t ci-github-jwt-validity-period = 600 { - "<minutes>", - "The number of minutes a JWT (authentication token) should be valid for. + "<seconds>", + "The number of seconds a JWT (authentication token) should be valid for. The maximum allowed by GitHub is 10 minutes." } }; |