Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 C++: Replace the remaing c code[WIP] Fixes #11 #21

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

S3 C++: Replace the remaing c code[WIP] Fixes #11 #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bf1bc1
Factor out s3_make_date by refactoring performOp
Thaodan Jun 28, 2020
ebefcec
Replace s3_hmac_sign() with HMAC() from OpenSSL
Thaodan Aug 2, 2020
b31c594
Refactor out the usage of the S3 struct
Thaodan Aug 2, 2020
6de0dee
Add braces to if signDataKey for consistency
Thaodan Aug 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 36 additions & 33 deletions src/s3access.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ extern "C" {

#include "s3access.h"

// Needed for performOp
#include <openssl/hmac.h>

S3Result::S3Result(QNetworkReply *reply, QObject *parent) : QObject(parent)
, m_reply(reply)
{
Expand All @@ -31,33 +34,18 @@ S3Result::~S3Result()
}

S3Access::S3Access(QObject *parent) : QObject(parent)
, m_s3(nullptr)
, m_manager(new QNetworkAccessManager(this))
{
}

S3Access::~S3Access()
{
if (m_s3) {
s3_free(m_s3);
m_s3 = nullptr;
}
}

void S3Access::initialise()
{
if (m_s3) {
s3_free(m_s3);
m_s3 = nullptr;
}
m_s3 = s3_init(m_id.toLatin1().data(), m_baseUrl.toLatin1().data(), m_baseUrl.toLatin1().data());
}

void S3Access::setId(QString const &id)
{
if (m_id != id) {
m_id = id;
initialise();
emit idChanged();
}
}
Expand All @@ -66,7 +54,6 @@ void S3Access::setSecret(QString const &secret)
{
if (m_secret != secret) {
m_secret = secret;
initialise();
emit secretChanged();
}
}
Expand All @@ -75,7 +62,6 @@ void S3Access::setBaseUrl(QString const &baseUrl)
{
if (m_baseUrl != baseUrl) {
m_baseUrl = baseUrl;
initialise();
emit baseUrlChanged();
}
}
Expand Down Expand Up @@ -110,37 +96,59 @@ QString S3Access::bucket() const

S3ListResult *S3Access::list(QString const &prefix)
{
char *date;
QString signData;
QString url;

QNetworkReply *reply;
S3ListResult *result;

date = s3_make_date();
signData = "GET\n\n\n" + QString(date) + "\n/" + m_bucket + "/";
url = "http://" + m_baseUrl + "/" + m_bucket + "/?delimiter=/";
if (!prefix.isEmpty()) {
url += "&prefix=" + prefix;
}

reply = performOp(GET, url, signData, date, nullptr, nullptr, nullptr);
reply = performOp(GET, url, nullptr, nullptr, nullptr);
result = new S3ListResult(reply, this);
free(date);

return result;
}

QNetworkReply *S3Access::performOp(Method method, QString const &url, QString const &sign_data, const char *date, QIODevice *in, const char *content_md5, const char *content_type)
QNetworkReply *S3Access::performOp(Method method, QString const &url, QIODevice *in, const char *content_md5, const char *content_type, QString signDataKey)
{
QNetworkRequest request;
char *digest;
unsigned char digest[EVP_MAX_MD_SIZE];
QByteArray digest_base64;
unsigned int out_length = 0;
QString methodStr;
QString date = QDateTime::currentDateTimeUtc().toString(Qt::RFC2822Date);
QNetworkReply *reply;

switch (method) {
case DELETE:
methodStr = "DELETE";
qDebug() << "DELETE request";
break;
case PUT:
methodStr = "PUT";
qDebug() << "PUT request";
break;
default: // GET
methodStr = "GET";
qDebug() << "GET request";
break;
}

QString signData = methodStr + "\n\n\n" + date + "\n/" + m_bucket + "/";

if (signDataKey != nullptr) {
signData+=signDataKey;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency (and because it's a common source of error) I'd be grateful if you could include the curly brackets here.

}
request.setUrl(QUrl(QString(url)));
digest = s3_hmac_sign(m_secret.toLatin1().data(), sign_data.toLatin1().data(), sign_data.toLatin1().size());
HMAC(EVP_sha1(), m_secret.toLatin1().data(), m_secret.toLatin1().size(), (unsigned char*)signData.toLatin1().data(), signData.toLatin1().size(), digest, &out_length);
digest_base64 = QByteArray((const char*)digest, out_length).toBase64();

request.setRawHeader("Date", date);
request.setRawHeader("Authorization", QString(QStringLiteral("AWS %1:%2")).arg(m_id).arg(digest).toLocal8Bit());
request.setRawHeader("Date", date.toLocal8Bit());
request.setRawHeader("Authorization", QString(QStringLiteral("AWS %1:%2")).arg(m_id).arg(digest_base64.data()).toLocal8Bit());
request.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);

qDebug() << "Request to: " << url;
Expand Down Expand Up @@ -168,7 +176,6 @@ QNetworkReply *S3Access::performOp(Method method, QString const &url, QString co
break;
}

free(digest);

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra line here after you've removed the free(digest). If you could remove it, that would be appreciated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see that line, I just removed that line here.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I wasn't very clear. I mean if you have this:

1.
2. statement
3.

and then remove statement, you end up with two blank lines:

1.
3.

but for purely aesthetic reasons there should only be a single blank line after the switch statement.

return reply;
}
Expand Down Expand Up @@ -220,19 +227,15 @@ QByteArray S3GetResult::data() const

S3GetFileResult *S3Access::getFile(QString const &key, QString const &filename)
{
char *date;
QString signData;
QString url;
QNetworkReply *reply;
S3GetFileResult *result;

date = s3_make_date();
signData = "GET\n\n\n" + QString(date) + "\n/" + m_bucket + "/" + key;
url = "http://" + m_baseUrl + "/" + m_bucket + "/" + key;

reply = performOp(GET, url, signData, date, nullptr, nullptr, nullptr);
reply = performOp(GET, url, nullptr, nullptr, nullptr, key);
result = new S3GetFileResult(reply, filename, this);
free(date);

return result;
}
Expand Down
3 changes: 1 addition & 2 deletions src/s3access.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,14 @@ public slots:
DELETE
};
void initialise();
QNetworkReply *performOp(Method method, QString const &url, QString const &sign_data, const char *date, QIODevice *in, const char *content_md5, const char *content_type);
QNetworkReply *performOp(Method method, QString const &url, QIODevice *in, const char *content_md5, const char *content_type, QString signDatakey = nullptr);

private:
QString m_id;
QString m_secret;
QString m_baseUrl;
QString m_bucket;

struct S3 *m_s3;
QNetworkAccessManager *m_manager;
};

Expand Down