Closed
Bug 412074
Opened 16 years ago
Closed 14 years ago
Ability to add attachments to a bug via the WebService (Bug.add_attachment)
Categories
(Bugzilla :: WebService, enhancement, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
(Whiteboard: [roadmap: 3.8])
Attachments
(1 file, 2 obsolete files)
8.95 KB,
patch
|
timello
:
review+
|
Details | Diff | Splinter Review |
The WebService should have the ability to add an attachment to a bug. Probably this depends on Bugzilla::Attachment using Bugzilla::Object->create().
Comment 1•16 years ago
|
||
We need to have a way to either throttle the data or limit the number of attachments that can be made using this method in a given time period or from a particular IP. I see the potential for abuse of this method if someone wishes to do harm to your bugzilla. It would take very little time if your attachment size limit is even as small as 1MB to fill your disk if someone wanted to launch a malicious upload script.
Assignee | ||
Comment 2•16 years ago
|
||
(In reply to comment #1) > It would take very little time if your attachment > size limit is even as small as 1MB to fill your disk if someone wanted to > launch a malicious upload script. That's no different from how things work now. You could do the same with WWW::Mechanize without the XML-RPC interface.
Comment 3•16 years ago
|
||
Selenium automatically uploads many files during QA tests. ;)
Comment 4•16 years ago
|
||
Hello, i am developer of a screenshot tool named „Bug Shooting“ (www.bugshooting.com). This tool can capture screen, edit screenshot and send it to a Bug Tracker system. At this time the application can send screenshots to Bug Tracker like FogBugz, Gemini, BugTracker.NET or OnTime. My dream ist o support BugZilla. So i need the feature to add an attachment to a bug too.
Assignee | ||
Comment 5•16 years ago
|
||
(In reply to comment #4) > Hello, i am developer of a screenshot tool named „Bug Shooting“ > (www.bugshooting.com). This tool can capture screen, edit screenshot and send > it to a Bug Tracker system. At this time the application can send screenshots > to Bug Tracker like FogBugz, Gemini, BugTracker.NET or OnTime. Great! We'd love to have that available for our users!
Priority: -- → P1
Summary: Ability to add attachments to a bug → Ability to add attachments to a bug via the WebService
Target Milestone: --- → Bugzilla 4.0
This is just what I'm looking for. Good luck Alexej! My team will be rooting for you.
Comment 7•16 years ago
|
||
Hello guys, the problem is i don't know how implement this feature in bugzilla. I need help from you all. So if someone is able to write the bugzilla code and want to see bugzilla support in Bug Shooting please contact me.
Assignee | ||
Updated•16 years ago
|
Whiteboard: [roadmap: 4.0]
Comment 9•15 years ago
|
||
(In reply to comment #8) > Currently working on this bug. timello, are you still working on it?
Comment 10•15 years ago
|
||
(In reply to comment #9) > (In reply to comment #8) > > Currently working on this bug. > > timello, are you still working on it? Not really, but I'm interested in. Just waiting for some time to back working on this bug.
Updated•14 years ago
|
Status: ASSIGNED → NEW
Updated•14 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [roadmap: 4.0] → [roadmap: 3.8]
Target Milestone: Bugzilla 4.0 → Bugzilla 3.8
Updated•14 years ago
|
Assignee: webservice → timello
Assignee | ||
Comment 11•14 years ago
|
||
I consider this to be a part of fully implementing Bug.update, so I'm going to work on it for Bugzilla 3.8. (LpSolit and I decided about a week ago that Bug.update stuff can continue to be implemented and reviewed through the freezes.)
Assignee: timello → mkanat
Assignee | ||
Comment 12•14 years ago
|
||
I consider this to be a part of fully implementing Bug.update, so I'm going to work on it for Bugzilla 3.8. (LpSolit and I decided about a week ago that Bug.update stuff can continue to be implemented and reviewed through the freezes.)
Assignee | ||
Comment 13•14 years ago
|
||
Okay, this allows both XML-RPC and JSON-RPC to add attachments to Bugzilla.
Attachment #453585 -
Flags: review?(timello)
Comment 14•14 years ago
|
||
Comment on attachment 453585 [details] [diff] [review] v1 >=== modified file 'Bugzilla/WebService.pm' >+# Used by the JSON-RPC server to convert incoming bsae64 fields appropriately. >+use constant BASE64_FIELDS => {}; base64 instead of bsae64. >+ my $attachment = Bugzilla::Attachment->create({ >+ bug => $bug, >+ data => $params->{data}, >+ description => $params->{summary}, >+ filename => $params->{file_name}, Why not $params->{filename} instead of $params->{file_name} to be consistent? >+ push(@created, $attachment); >+ } >+ $_->update() foreach @bugs; >+ $dbh->bz_commit_transaction(); >+ >+ $_->send_changes() foreach @bugs; Why not have $_->update be $bug->update within the foreach loop? I can understand send_changes needs to be outside of the transaction but the update? >+=item B<Params> >+ >+=over >+ >+=item C<ids> >+ >+B<Required> C<array> An array of ints or strings--the ids Nit: s/ints/integers/. Also maybe say and/or since the list can be a mix of integers and strings. >+=item C<is_private> >+ >+C<boolean> True if the attachment should be private (restricted >+to the "insidergroup"), False if the attachment should be public. >+ >+=item C<is_url> >+ >+C<boolean> True if the attachment is just a URL, pointing to data elsewhere. >+If so, the C<data> item should just contain the URL. Should denote that they default to 'false' if omitted. >+=item 600 (Attachment Too Large) >+ >+You tried to attach a file that was than Bugzilla will accept. You tried to attach a file that was *larger* than Bugzilla will accept. >=== modified file 'Bugzilla/WebService/Server/JSONRPC.pm' >+ my @base64_fields = @{ $pkg->BASE64_FIELDS->{$method} || [] }; >+ foreach my $field (@base64_fields) { >+ if (defined $params->{$field}) { >+ $params->{$field} = decode_base64($params->{$field}); >+ } >+ } What does this do if the 'data' argument is a URL and not base64 encoded? Does it just leave it alone? Dave
Comment 15•14 years ago
|
||
Comment on attachment 453585 [details] [diff] [review] v1 r- due Dave comments.
Attachment #453585 -
Flags: review?(timello) → review-
Assignee | ||
Comment 16•14 years ago
|
||
(In reply to comment #14) > Why not have $_->update be $bug->update within the foreach loop? I can > understand > send_changes needs to be outside of the transaction but the update? update() is supposed to happen after all validation is done, to avoid rollbacks. (That is, in general, rollbacks are never supposed to happen.) I would be doing the same with Attachment->create, but we don't have an architecturally-ideal way to do that right now. > Nit: s/ints/integers/. Oh, I actually think that I'm going to keep that as "ints", since it's a technical type name for the WebService. > What does this do if the 'data' argument is a URL and not base64 encoded? > Does it just leave it alone? Under JSON-RPC, you will always have to pass the data argument as base64-encoded, since JSON-RPC doesn't have an explicit base64 type. The parameter is documented as base64, so it's not inconsistent with its documentation.
Assignee | ||
Comment 17•14 years ago
|
||
Oh, and also, file_name is consistent with Bug.attachments. "summary" was not, so I changed Bug.attachments and left in "description" for backwards-compat. New patch coming.
Assignee | ||
Comment 18•14 years ago
|
||
Okay, this fixes all of Dave's comments and various consistency issues.
Attachment #453585 -
Attachment is obsolete: true
Attachment #456133 -
Flags: review?(timello)
Assignee | ||
Updated•14 years ago
|
Flags: blocking4.0+
Comment 19•14 years ago
|
||
Comment on attachment 456133 [details] [diff] [review] v2 Attachment->create() does not except base64 data type, you probably forgot to call decode_base64 in the add_attachment XMLRPC method. Also I think 'is_patch' parameter is missing. Nit: The error is saying: "You must enter a description for the attachment." but the documentation says 'summary' instead. I know this error msg is shared between the webservices and the cgis too... just mentioning here.
Attachment #456133 -
Flags: review?(timello) → review-
Comment 20•14 years ago
|
||
Ok. I was not using SOAP::Data->type(base64 => $data) when passing the data. It doesn't need the decode in the XMLRPC method.
Assignee | ||
Comment 21•14 years ago
|
||
Okay, this adds is_patch and refactors slightly to avoid more rollbacks, and also I made sure that the comment now "happens" at the exact same time as the attachment was added.
Attachment #456133 -
Attachment is obsolete: true
Attachment #456377 -
Flags: review?(timello)
Comment 22•14 years ago
|
||
not ok 178 - WS_ERROR_CODE has 2 error(s): # Error tag 'local_file_too_large' is used in WS_ERROR_CODE in Bugzilla/WebService/Constants.pm but not defined in any template, and not used in any code. # Error tag 'patch_too_large' is used in WS_ERROR_CODE in Bugzilla/WebService/Constants.pm but not defined in any template, and not used in any code. # Failed test 'WS_ERROR_CODE has 2 error(s): # Error tag 'local_file_too_large' is used in WS_ERROR_CODE in Bugzilla/WebService/Constants.pm but not defined in any template, and not used in any code. # Error tag 'patch_too_large' is used in WS_ERROR_CODE in Bugzilla/WebService/Constants.pm but not defined in any template, and not used in any code.' # at t/012throwables.t line 208.
Assignee | ||
Comment 23•14 years ago
|
||
That will only happen on trunk, though. That's due to a patch that was checked in after I wrote this patch.
Assignee | ||
Updated•14 years ago
|
Attachment #456377 -
Attachment description: v3 → v3 (4.0)
Attachment #456377 -
Flags: review?(timello) → review?(dkl)
Comment 24•14 years ago
|
||
I tested it using XML-RPC and it is working fine. I still need to check the behavior with JSON, thought, I'm ok with the patch.
Comment 25•14 years ago
|
||
I tested it using JSON-RPC. I could successful send attachment in text/plain type but it fails when sending binaries like image/jpg, application/octet-stream... I'm using encode_base64($data) for both cases... reading the file as: open($fh, '/home/timello/file...' ) or die $!; read( $fh, $data, -s $fh ); The same code works for XML-RPC (apart from the encode_base64 which is unnecessary...) When it does the my $result = $rpc->call($uri, {.... , it doesn't even return any object instance ... it looks like it fails in the $rpc->call or something... maybe the string with the base64 content is too long?
Assignee | ||
Comment 26•14 years ago
|
||
timello: See the docs about line breaks in base64, in the patch.
Comment 27•14 years ago
|
||
Comment on attachment 456377 [details] [diff] [review] v3 (4.0) Ok. I removed all newlines and it worked. I sniffed the traffic and used the response as a positive answer for the review since it contains all expected returning data. I'm ok with the patch.
Attachment #456377 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Flags: approval4.0+
Flags: approval+
Assignee | ||
Updated•14 years ago
|
Attachment #456377 -
Flags: review?(dkl)
Assignee | ||
Comment 28•14 years ago
|
||
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/Attachment.pm modified Bugzilla/WebService.pm modified Bugzilla/WebService/Bug.pm modified Bugzilla/WebService/Constants.pm modified Bugzilla/WebService/Server/JSONRPC.pm Committed revision 7350. Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/ modified Bugzilla/Attachment.pm modified Bugzilla/WebService.pm modified Bugzilla/WebService/Bug.pm modified Bugzilla/WebService/Constants.pm modified Bugzilla/WebService/Server/JSONRPC.pm Committed revision 7319.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Summary: Ability to add attachments to a bug via the WebService → Ability to add attachments to a bug via the WebService (Bug.add_attachment)
Comment 29•14 years ago
|
||
Please correct me if I'm interpreting the bug incorrectly. The request was to add the ability to add attachments to a bug via the WebService. I was interpreting this to mean something like inline pasting of an image. I'm imagining I'm a software user with a problem, and I want to provide a screen capture of the problem I'm seeing. I'd like to be able to insert that image straight into the incident (copy/paste) so it's viewable as part of the bug description. This would be similar to what one can do in Jira, and would be accomplished using something like "KSnapshot" or BugShooting" (see comment #4 above). The status of this request is "Resolved Fixed" with roadmap of 3.8? I just logged into the 4.1 landfill, but I do not not see that that I can insert a screen capture. I do see that I can attach a supporting document (OK, but clumsy), but that's not what I thought this bug was addressing. Am I reading this wrong then? Should the ability to do so exist in this demo veeersion of software? Thanks much for setting me straight.
Comment 30•14 years ago
|
||
(In reply to comment #29) > Please correct me if I'm interpreting the bug incorrectly. You are! :) This bug has nothing to do with the web UI. What you are talking about is bug 602313.
Assignee | ||
Comment 31•14 years ago
|
||
(In reply to comment #29) > The request was to add the ability to add attachments to a bug via the > WebService. I was interpreting this to mean something like inline pasting of an > image. To be clear, a "WebService" is a remote procedure call service like JSON-RPC, XML-RPC, or some form of REST API.
Comment 33•10 years ago
|
||
Hi, By using add_attachment method I can upload files with different extensions (.zip, .jpg etc) , but uploaded files got corrupted. I've google it and found that many people have this problem with add_attachment method of Bugzilla API. Ref: http://stackoverflow.com/questions/7131197/uploading-attachments-to-bugzilla-using-the-web-services-api-and-perl http://stackoverflow.com/questions/13516982/add-an-attachment-to-bugzilla-using-xml-rpc-in-vba My Bugzilla version is 4.2.3 I am using a python module for interacting with Bugzilla over XMLRPC https://git.fedorahosted.org/git/python-bugzilla.git I am just copying here the attachfile function from base.py Its very straightforward python code. def attachfile(self, idlist, attachfile, description, **kwargs): if isinstance(attachfile, str): f = open(attachfile) elif hasattr(attachfile, 'read'): f = attachfile else: raise TypeError("attachfile must be filename or file-like object") # Back compat if "contenttype" in kwargs: kwargs["content_type"] = kwargs.pop("contenttype") if "ispatch" in kwargs: kwargs["is_patch"] = kwargs.pop("ispatch") if "isprivate" in kwargs: kwargs["is_private"] = kwargs.pop("isprivate") if "filename" in kwargs: kwargs["file_name"] = kwargs.pop("filename") kwargs['summary'] = description if 'file_name' not in kwargs: kwargs['file_name'] = os.path.basename(f.name) if 'content_type' not in kwargs: kwargs['content_type'] = 'application/octet-stream' kwargs['data'] = self._attachment_encode(f) kwargs['ids'] = self._listify(idlist) ret = self._proxy.Bug.add_attachment(kwargs) if "attachments" in ret: # Up to BZ 4.2 ret = [int(k) for k in ret["attachments"].keys()] elif "ids" in ret: # BZ 4.4+ ret = ret["ids"] if type(ret) is list and len(ret) == 1: ret = ret[0] return ret Is this the correct way to write this comment here on already Resolved bug or shall I report a separate bug ?
Comment 34•10 years ago
|
||
Please open a new fresh bug for this issue and we can track it there. Also in the new bug please provide some sample attachments that are getting corrupted for us to reproduce. (In reply to Khokhar from comment #33) > > kwargs['data'] = self._attachment_encode(f) I assume this is base64 encoding the data before sending? dkl
Comment 35•10 years ago
|
||
(In reply to David Lawrence [:dkl] from comment #34) > Please open a new fresh bug for this issue and we can track it there. Also > in the new bug please provide some sample attachments that are getting > corrupted for us to reproduce. Thanks for the quick reply new fresh bug#958559 has been created. I've uploaded sample attachments as well.
You need to log in
before you can comment on or make changes to this bug.
Description
•