Implement XEP-0363: HTTP File Upload: Request IQs #188
Conversation
Codecov Report
@@ Coverage Diff @@
## master #188 +/- ##
=========================================
Coverage ? 66.26%
=========================================
Files ? 151
Lines ? 15015
Branches ? 0
=========================================
Hits ? 9949
Misses ? 5066
Partials ? 0
Continue to review full report at Codecov.
|
lnjX
force-pushed
the
feature/http-file-upload
branch
from
9a5ea1c
to
8751c76
Compare
|
added tests for |
doc/xep.doc
Outdated
| @@ -46,5 +46,6 @@ Ongoing: | |||
| - XEP-0009: Jabber-RPC (API is not finalized yet) | |||
| - XEP-0060: Publish-Subscribe (Only basic IQ implemented) | |||
| - XEP-0077: In-Band Registration (Only basic IQ implemented) | |||
| - XEP-0363: HTTP File Upload [v0.9.0] | |||
There was a problem hiding this comment.
What does the version refer to?
There was a problem hiding this comment.
It's the XEP's version, so we can easily check if an implementation is outdated.
There was a problem hiding this comment.
I also added the version number in the header files, maybe that's a bit too much?
lnjX
force-pushed
the
feature/http-file-upload
branch
2 times, most recently
from
4403e56
to
e2b59a9
Compare
|
Now with a d-pointer. |
lnjX
force-pushed
the
feature/http-file-upload
branch
from
e2b59a9
to
510320f
Compare
src/base/QXmppHttpUploadIq.cpp
Outdated
| for (QString &name : putHeaders.keys()) { | ||
| if (name == "Authorization" || name == "Cookie" || name == "Expires") { | ||
| // newlines must be removed | ||
| QString value = putHeaders[name]; |
There was a problem hiding this comment.
I'm not convinced "removing" newlines is the best option. Also I expect \r isn't accepted either?
Wouldn't failing loudly be more appropriate?
There was a problem hiding this comment.
The XEP says we must remove the newlines:
The requesting entity MUST strip any newline characters from the header name and value [...].
Would \r work on its own (without \n before)?
There was a problem hiding this comment.
I'm not sure, but maybe Qt is already doing this anyway, so we might don't need to remove newlines on our own.
|
|
||
| /// Returns the file's size in bytes. | ||
|
|
||
| qint64 QXmppHttpUploadRequestIq::size() const |
There was a problem hiding this comment.
I'm sorry for pointing out these things one by one.. but why fileName() vs size() ? I would expect either name() and size() or fileName() and fileSize().
QXmppTransferFileInfo use name() and size()
Ah.. actually forget this I guess it comes from the XML attribute names.
There was a problem hiding this comment.
Yeah right, I took the XML attr. names.
There was a problem hiding this comment.
Is that ok or should I better use consistent names?
|
Could you rebase this please? Also, will there be a manager to handle these IQs? |
This implements the IQs for requesting and receiving upload slots as defined by XEP-0363: HTTP File Upload in version 0.9.0.
lnjX
force-pushed
the
feature/http-file-upload
branch
from
510320f
to
fbe8411
Compare
|
Done. Yes there will be a manager or even two: One for requesting/receiving the slots from the server and one for actually uploading a file from disk. |
|
Thank you!
And this really helps. |
This implements the IQs for requesting and receiving upload slots as
defined by XEP-0363: HTTP File Upload [v0.9.0].