From 25a15466a898940a5dac69b05ce356e8b1c692a3 Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Wed, 20 Jun 2018 19:23:01 -0700 Subject: [PATCH 1/8] SG-4483: Handles non-ascii unicode string paths gracefully during upload. --- shotgun_api3/shotgun.py | 14 ++++++++++++++ "tests/No\303\253l.jpg" | Bin 0 -> 11361 bytes tests/test_api.py | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 "tests/No\303\253l.jpg" diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 4a54e72a..2591dfbc 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2365,6 +2365,20 @@ def _upload_to_sg(self, entity_type, entity_id, path, field_name, display_name, params.update(self._auth_params()) + # If we ended up with a unicode string path, we need to encode it + # as a utf-8 string. If we don't, there's a chance that there will + # will be an attempt later on to encode it as an ascii string, and + # that will fail ungracefully if the path contains any non-ascii + # characters. + if isinstance(path, unicode): + try: + path = path.encode("utf-8") + except UnicodeEncodeError: + raise ShotgunError( + "Could not upload file successfully. The unicode file path given must " + "be encodable as utf-8 in order to be uploaded to Shotgun." + ) + if is_thumbnail: url = urlparse.urlunparse((self.config.scheme, self.config.server, "/upload/publish_thumbnail", None, None, None)) diff --git "a/tests/No\303\253l.jpg" "b/tests/No\303\253l.jpg" new file mode 100644 index 0000000000000000000000000000000000000000..7a8aaf9677c755850ca6a0419e69de44a9c8a153 GIT binary patch literal 11361 zcmeG?c|cUv*Y~|Q3j@PCfVe+~RTO51ol#^NwoyiK7(mU^;mu-o59ZbMCq4p1Ys9 zo##5+IJvoXhDrcbDi8wz@Bn-)01yzu;2*&H0WJ#zP=WP#!&9*6E*>26$N-E%IQ(o_ zJPQXxxZ!C7;B=LJ6{hEa=iONVMESYWES;r-CX1^qjKyAMv62b}884NklF=$HWiTV; z5P%SJX|ga4!)9IN>7t z0gQtYI0xtQc^+Ism;j0X1Sd2=#?2b56^W ztAl}AwDN#rJ5y3&u^3qnrJ12=v%`c42u7*GXhV5lwrq)^4sm1~7*qz4oib_L7_Euc zEt6v@(jx+;Z9#w_3*Z{92pZNU>*&c5#aAXe%PHZNdf z7-4KFxILrb*)>VvfMt0*SZo21R8~TY5yo&cg#qirtY5jxjX^s!1}4DR#goKh(8%w? zurhyvd4OhCXRm>{g2=s+I{zm$))pQ}wrd{ZnXsAg>$2M*1hGc|rycvDETM#vQ+t#J za5r^+o}d(%;TmCppAoJWIuHwFkk&PQ5UV@%#JS45i3sS);39M{7QpN~}dsv31UKm*)8ieUb}Q3$fTM=MeVsJjOv%Jb|-14Y}*jD=`AVfWO-WoZx{ z06#vPQReihi?6p?9M*eKJd4esH@H{I+!7?ch{Zt}-dYD^$*0Y236uZp_7e`9@t*C1HU7OAm0f@Dmcr8-8I4wd50?)`8;u&ej#1hRG^#?^ zqUGU2-@`1Zve>3)84Y@urF@-@FL0q@%lSIGQtMzBQ9%uDV|rmPbD{5L_o~nSMx%=0 zZVQKVMmq~TXB)aYIEOIC@*xgDguw_+4m3F6hYbV3A%%E82!V&}hllWohwz7oaOeC9 zg);3AjEmci3WCacPf^_kZ zZ@(`l4Z3u3d6G)1vMT5*L&0<#T{3-)hMHbYrRl^OBgqkU@;Z~%L^E2l&SW&(<#p*| zR=6C(2rUtlEDKYeE@rRZS+X=SNj40*7*ZtCB#AUR zRw|QAQ|0k8vMY#TZ8lw{T%D8GRTs>qi@O?CTU#4fn-FKQ=_RtXv^0q{UJ@T43prx# z)69&vF4k-hb4kdd?Uc>Hp65v<5uQ~YOu86q>TH6^>K5Cptz3*c%FSnm8xmW&j*`$u z+C-ZfJG83|HQyyvrE;_PD%NCjbK9BR8mQ4WGRom&HI40Baxb$M-i z7vUzeJ&y`m2xu22F1dDib1akt^`oo2?!Q^y zd4AFw|G6A@KgrqsUuDrjGbf12ZoE>VfcM!-gAo$!@GeLya}~+5v}D)|aWXXAVZ)&* z3JiAGx6`^NPzgP?3o;r=cBIL*6uR7|izze;Xq~tVZTQC~bc(HoSF*nx7*003rIM-D z+UP7jH22>deG=FL37Wgk#z^jOCrj%w&t2?r@j#$^)f(wqT^^Ii~)dpSj zSTW>J(7_uFgy%unSj$)uz8%7W71apFIS95v1tA=QV7(KDei_K)gfkJWGnt{k21hzu zbtWBx4?*~u8V7Rp5rq(*Ut^$aA$$tL;YNqa0Ab`C4m8nP=zQcL4^TKmQ&kX_LRe_i zsG)D4GXnlvE!6jbD|)~T^teG@ps-k{u|6m=ni@jFi(4vLK-U^+hKVhPmr<=v2d|1I ztJXXXI+oZrqf#K?4`*E#?Tr4C*n{t{K-9NR`i-${pJBmAyK&tmyK$CA=%0puc;fkP zT*XQN_B;hZ$miX-@OALkKM#Q2$9uFV7>$=67g+;M#UX>b!vh)IoPQ4Vs4ouH=e{S9 zIZl@)iAFUAU62kNX@|Z5nvA`F5dY(XJ=E%<$1pWrN!w_6Nh8bPD1$%T!0tBd4CpdK z8qD|HW_n2YhsAnGK%jG6LqN3gcMv!!4)`4p1jP480Y~Hoh>0&j3g+&&LO~gH&j3(9 z__A}{Lm1N8_?`$~470f1z`DVKLaiYwhpooR!a)fa{(9pJ0zqFe2!w%X_Zd@4R2 zpNBt(ufQAeSMlBW0sIsEG~SAThj+l&99~2)5lX}osf2M`o+~`Jdmi@u!t;ig zmzUTp&14N< zg`d{%Nx#*8Z~2||>+l!(%lwP{P5z7gxB7qNeV_SxR2rO&O9{vnwmbjX5`9U-SfZuceo=Ju`byQJ@)zF+p^^o!{? zwqI?(m--#%32d*4=aNxB; z!GkgfO&PRo(0hZf4i*g_IoL3G+2HpFe@h0DS)`F%K^`L8LkENwggQdkhaL;PBaRZ6 zisy)Th`$W;3`-5G3|kh~6xJR-I9wGzBYa!<=MjR4)QGBx zFz&FlVdi0t!_LMEW3yswVzzxR$$pBDiKpUM$Dc~@PRL7`nXotEdg9PTI`O5%b4j8kRnpT*O-a8crzG2wwY~&S)3|BbX*1K_N&8ivENA39+mZhqDIgoHjcQM zE>72`uTO6sNsgR6a^1+*jL-~iMq|c>%&<&d=BCWcSwpg>WWAd8L$);Amc1+cmO`$0 zLa{%G$jQ%HkaH|IAh$GkRqj`L;yiucw!HTIr2NP8_ZM&rMi(q8IIA3}q?B8f?W0mg zJu&LgXyNFR(W^#ZD2yqz74B2vs!^(?sxQVwj4_YdQv`~XMN5mmERHIkTKx7{-q_-? ztH*w$mZ@i`kCcc?CY5Y0xv9z4EY_SajV^VRHkJ97O)T3|cB?$6yuQ43oMhY+<31VR zZ@gjr-U<8(WfL|{=y)Xekry7hG%;!7{E25Cje4~1(T^VM_n7gqcPIHy(oT9~GIw(M z2y7PxiY=7zVeDbUB6U+wJM{kq3W7J zVOV9jKBZvF+9|iHi>fzQcN)u$J4_y?$)>l=0p@D+AhVI#oXPg{kef z(Y8%?%>Jl-FB1q4Q%4=qj;9=LH6v?Q*ZfjjR{KVsf1SDR=(Lz=&rZ8MJ#YHUk8>WU z9&efvI%D>XwkNWlSpOvUWW|$BGs9-in|Wzg{;XGKd(N(&{plR(occLG&n=(3Z{EOp zv*ulVD*vf%^L^&q=AV6f_|t2j;XGq_=Ge0d&#rv-_XWBIA1{&~su zU%$ICvaw+UZ$r(7Z#I@~Jo0kt%Ud?}+q7`gZ=21VTVGMXa%fA+mMvQcY+bw!-{#nM z_0@^5p7?jpzxQsB-M)TD@Q#JAf!7?bef#?4*U#)6y|d|!)Hk-j8S&-No;DzBli^&iALk-?87i|Jnil zfs0L(n$8~_cktAqv4@Tw9)0-35Ar@ZcqHq{`yZx%_|8Z2kKXz?_2WICB!9B|XwuPL zpC)~}>sa!!-N#do?>&)rVqf!!=J#4MS`M7dIr+gU<*83jk2&3Zru59YvlGv@p3|MX z@|p3o>z~(re&_tGFSuVU_|osohOY*E)%c$w|Jl)+-1=@?PTSE7B^S7mY9W?-kC~+SY#DC}gASO;oNQ_JrhlGg#_a7kk zheulH`$HJB#~Aoip8JiVn}Pr*wVUXNUjGp=&*u>Y568M*{{_h4>p!tbI~6|udp&zS zxA{O|yE@F4P|-H$x+1Z@ac59VNw~d<;>}%p;inBb8eK$EZg3=%TuQs1AO88l5ZE!W(xcmgG?O zrk1%&H)t;W#0`RP40r8g6NEP#5Uid7^i8bMNSu-q9y} oN1y5)efl Date: Thu, 21 Jun 2018 09:53:48 -0700 Subject: [PATCH 2/8] Removes unneeded exception handling, and attempts to fix a test. --- shotgun_api3/shotgun.py | 8 +------- tests/test_api.py | 3 ++- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 2591dfbc..7eefb8aa 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2371,13 +2371,7 @@ def _upload_to_sg(self, entity_type, entity_id, path, field_name, display_name, # that will fail ungracefully if the path contains any non-ascii # characters. if isinstance(path, unicode): - try: - path = path.encode("utf-8") - except UnicodeEncodeError: - raise ShotgunError( - "Could not upload file successfully. The unicode file path given must " - "be encodable as utf-8 in order to be uploaded to Shotgun." - ) + path = path.encode("utf-8") if is_thumbnail: url = urlparse.urlunparse((self.config.scheme, self.config.server, diff --git a/tests/test_api.py b/tests/test_api.py index 3ec0a4cf..5ce258b5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -15,6 +15,7 @@ import urlparse import urllib2 import warnings +import glob import shotgun_api3 from shotgun_api3.lib.httplib2 import Http, SSLHandshakeError @@ -222,7 +223,7 @@ def test_upload_download(self): u_path = unicode( os.path.abspath( os.path.expanduser( - os.path.join(this_dir, 'Noe\xcc\x88l.jpg') + glob.glob(os.path.join(this_dir, 'No*l.jpg'))[0] ) ), "utf-8" From 5792d8c0733de229f5f2f23c3661578c0eb45606 Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Thu, 21 Jun 2018 10:25:36 -0700 Subject: [PATCH 3/8] Tweaks a test to address Windows failures. --- tests/test_api.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 5ce258b5..098752c4 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -220,13 +220,10 @@ def test_upload_download(self): self.assertRaises(TypeError, self.sg.download_attachment) # test upload of non-ascii, unicode path - u_path = unicode( - os.path.abspath( - os.path.expanduser( - glob.glob(os.path.join(this_dir, 'No*l.jpg'))[0] - ) - ), - "utf-8" + os.path.abspath( + os.path.expanduser( + glob.glob(os.path.join(unicode(this_dir), u'No*l.jpg'))[0] + ) ) # If this is a problem, it'll raise with a UnicodeEncodeError. We From 3ac496a8e6517e30f7533bafa8ae3a26f2c87f4f Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Thu, 21 Jun 2018 10:46:25 -0700 Subject: [PATCH 4/8] Adds in a missing variable assignment in a test because I'm an idiot. --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 098752c4..0272d3ee 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -220,7 +220,7 @@ def test_upload_download(self): self.assertRaises(TypeError, self.sg.download_attachment) # test upload of non-ascii, unicode path - os.path.abspath( + u_path = os.path.abspath( os.path.expanduser( glob.glob(os.path.join(unicode(this_dir), u'No*l.jpg'))[0] ) From 1a32e5029fa287fd3076ab3d80a3b5e439d53f6e Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Thu, 21 Jun 2018 13:06:16 -0700 Subject: [PATCH 5/8] Addresses issues on Windows with unicode file paths. --- shotgun_api3/shotgun.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 7eefb8aa..df09cc02 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2370,13 +2370,22 @@ def _upload_to_sg(self, entity_type, entity_id, path, field_name, display_name, # will be an attempt later on to encode it as an ascii string, and # that will fail ungracefully if the path contains any non-ascii # characters. + # + # On Windows, if the path contains non-ascii characters, the calls + # to open later in this method will fail to find the file if given + # a utf-8 encoded string path. In that case, we're going to have + # to call open on the unicode path, but we'll use the utf-8 encoded + # string for everything else. + path_to_open = path if isinstance(path, unicode): path = path.encode("utf-8") + if sys.platform != "win32": + path_to_open = path if is_thumbnail: url = urlparse.urlunparse((self.config.scheme, self.config.server, "/upload/publish_thumbnail", None, None, None)) - params["thumb_image"] = open(path, "rb") + params["thumb_image"] = open(path_to_open, "rb") if field_name == "filmstrip_thumb_image" or field_name == "filmstrip_image": params["filmstrip"] = True @@ -2393,7 +2402,7 @@ def _upload_to_sg(self, entity_type, entity_id, path, field_name, display_name, if tag_list: params["tag_list"] = tag_list - params["file"] = open(path, "rb") + params["file"] = open(path_to_open, "rb") result = self._send_form(url, params) @@ -3937,7 +3946,15 @@ def encode(self, params, files, boundary=None, buffer=None): buffer.write('Content-Disposition: form-data; name="%s"' % key) buffer.write('\r\n\r\n%s\r\n' % value) for (key, fd) in files: - filename = fd.name.split('/')[-1] + # On Windows, it's possible that we were forced to open a file + # with non-ascii characters as unicode. In that case, we need to + # encode it as a utf-8 string to remove unicode from the equation. + # If we don't, the mix of unicode and strings going into the + # buffer can cause UnicodeEncodeErrors to be raised. + filename = fd.name + if isinstance(filename, unicode): + filename = filename.encode("utf-8") + filename = filename.split('/')[-1] content_type = mimetypes.guess_type(filename)[0] content_type = content_type or 'application/octet-stream' file_size = os.fstat(fd.fileno())[stat.ST_SIZE] From e39f2d034539f8da6f2308f2e773e5a129e564fb Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Mon, 25 Jun 2018 12:06:55 -0700 Subject: [PATCH 6/8] More graceful handling of non-ascii/utf-8 string encodings, and handles non-ascii string paths on Windows. --- shotgun_api3/shotgun.py | 24 +++++++++++++++++++++--- tests/test_api.py | 29 +++++++++++++++++++++++++++++ "tests/\343\201\224.shift-jis" | Bin 0 -> 11361 bytes 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 "tests/\343\201\224.shift-jis" diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index df09cc02..ae34ae65 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2251,6 +2251,24 @@ def upload(self, entity_type, entity_id, path, field_name=None, display_name=Non """ # Basic validations of the file to upload. path = os.path.abspath(os.path.expanduser(path or "")) + + # On Windows, if a path is given as a string encoded as something other + # than ascii and it contains non-ascii characters, the os.path calls to + # determine its validity will return a false negative. Likewise, later + # in the upload process, opening a filehandle will fail in the same way. + # To combat this, we'll try to decode the string path into unicode, + # which Windows handles well, and that we have proper support for in + # upload pipeline here. + if not isinstance(path, unicode) and sys.platform == "win32": + try: + path = path.decode("utf-8") + except UnicodeDecodeError: + raise ShotgunError( + "Could not upload the given file path. It is encoded as " + "something other than utf-8 or ascii. To upload this file, " + "it can be decoded into unicode prior to upload: %s" % path + ) + if not os.path.isfile(path): raise ShotgunError("Path must be a valid file, got '%s'" % path) if os.path.getsize(path) == 0: @@ -2373,9 +2391,9 @@ def _upload_to_sg(self, entity_type, entity_id, path, field_name, display_name, # # On Windows, if the path contains non-ascii characters, the calls # to open later in this method will fail to find the file if given - # a utf-8 encoded string path. In that case, we're going to have - # to call open on the unicode path, but we'll use the utf-8 encoded - # string for everything else. + # a non-ascii-encoded string path. In that case, we're going to have + # to call open on the unicode path, but we'll use the encoded string + # for everything else. path_to_open = path if isinstance(path, unicode): path = path.encode("utf-8") diff --git a/tests/test_api.py b/tests/test_api.py index 0272d3ee..d67386b5 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -238,6 +238,35 @@ def test_upload_download(self): tag_list="monkeys, everywhere, send, help" ) + # Also make sure that we can pass in a utf-8 encoded string path + # with non-ascii characters and have it work properly. This is + # primarily a concern on Windows, as it doesn't handle that + # situation as well as OS X and Linux. + self.sg.upload( + "Ticket", + self.ticket['id'], + u_path.encode("utf-8"), + 'attachments', + tag_list="monkeys, everywhere, send, help" + ) + + # Make sure that non-utf-8 encoded paths raise on Windows. + if sys.platform == "win32": + u_path = os.path.abspath( + os.path.expanduser( + glob.glob(os.path.join(unicode(this_dir), u'*.shift-jis'))[0] + ) + ) + self.assertRaises( + shotgun_api3.ShotgunError, + self.sg.upload, + "Ticket", + self.ticket['id'], + u_path.encode("shift-jis"), + 'attachments', + tag_list="monkeys, everywhere, send, help" + ) + # cleanup os.remove(file_path) diff --git "a/tests/\343\201\224.shift-jis" "b/tests/\343\201\224.shift-jis" new file mode 100644 index 0000000000000000000000000000000000000000..7a8aaf9677c755850ca6a0419e69de44a9c8a153 GIT binary patch literal 11361 zcmeG?c|cUv*Y~|Q3j@PCfVe+~RTO51ol#^NwoyiK7(mU^;mu-o59ZbMCq4p1Ys9 zo##5+IJvoXhDrcbDi8wz@Bn-)01yzu;2*&H0WJ#zP=WP#!&9*6E*>26$N-E%IQ(o_ zJPQXxxZ!C7;B=LJ6{hEa=iONVMESYWES;r-CX1^qjKyAMv62b}884NklF=$HWiTV; z5P%SJX|ga4!)9IN>7t z0gQtYI0xtQc^+Ism;j0X1Sd2=#?2b56^W ztAl}AwDN#rJ5y3&u^3qnrJ12=v%`c42u7*GXhV5lwrq)^4sm1~7*qz4oib_L7_Euc zEt6v@(jx+;Z9#w_3*Z{92pZNU>*&c5#aAXe%PHZNdf z7-4KFxILrb*)>VvfMt0*SZo21R8~TY5yo&cg#qirtY5jxjX^s!1}4DR#goKh(8%w? zurhyvd4OhCXRm>{g2=s+I{zm$))pQ}wrd{ZnXsAg>$2M*1hGc|rycvDETM#vQ+t#J za5r^+o}d(%;TmCppAoJWIuHwFkk&PQ5UV@%#JS45i3sS);39M{7QpN~}dsv31UKm*)8ieUb}Q3$fTM=MeVsJjOv%Jb|-14Y}*jD=`AVfWO-WoZx{ z06#vPQReihi?6p?9M*eKJd4esH@H{I+!7?ch{Zt}-dYD^$*0Y236uZp_7e`9@t*C1HU7OAm0f@Dmcr8-8I4wd50?)`8;u&ej#1hRG^#?^ zqUGU2-@`1Zve>3)84Y@urF@-@FL0q@%lSIGQtMzBQ9%uDV|rmPbD{5L_o~nSMx%=0 zZVQKVMmq~TXB)aYIEOIC@*xgDguw_+4m3F6hYbV3A%%E82!V&}hllWohwz7oaOeC9 zg);3AjEmci3WCacPf^_kZ zZ@(`l4Z3u3d6G)1vMT5*L&0<#T{3-)hMHbYrRl^OBgqkU@;Z~%L^E2l&SW&(<#p*| zR=6C(2rUtlEDKYeE@rRZS+X=SNj40*7*ZtCB#AUR zRw|QAQ|0k8vMY#TZ8lw{T%D8GRTs>qi@O?CTU#4fn-FKQ=_RtXv^0q{UJ@T43prx# z)69&vF4k-hb4kdd?Uc>Hp65v<5uQ~YOu86q>TH6^>K5Cptz3*c%FSnm8xmW&j*`$u z+C-ZfJG83|HQyyvrE;_PD%NCjbK9BR8mQ4WGRom&HI40Baxb$M-i z7vUzeJ&y`m2xu22F1dDib1akt^`oo2?!Q^y zd4AFw|G6A@KgrqsUuDrjGbf12ZoE>VfcM!-gAo$!@GeLya}~+5v}D)|aWXXAVZ)&* z3JiAGx6`^NPzgP?3o;r=cBIL*6uR7|izze;Xq~tVZTQC~bc(HoSF*nx7*003rIM-D z+UP7jH22>deG=FL37Wgk#z^jOCrj%w&t2?r@j#$^)f(wqT^^Ii~)dpSj zSTW>J(7_uFgy%unSj$)uz8%7W71apFIS95v1tA=QV7(KDei_K)gfkJWGnt{k21hzu zbtWBx4?*~u8V7Rp5rq(*Ut^$aA$$tL;YNqa0Ab`C4m8nP=zQcL4^TKmQ&kX_LRe_i zsG)D4GXnlvE!6jbD|)~T^teG@ps-k{u|6m=ni@jFi(4vLK-U^+hKVhPmr<=v2d|1I ztJXXXI+oZrqf#K?4`*E#?Tr4C*n{t{K-9NR`i-${pJBmAyK&tmyK$CA=%0puc;fkP zT*XQN_B;hZ$miX-@OALkKM#Q2$9uFV7>$=67g+;M#UX>b!vh)IoPQ4Vs4ouH=e{S9 zIZl@)iAFUAU62kNX@|Z5nvA`F5dY(XJ=E%<$1pWrN!w_6Nh8bPD1$%T!0tBd4CpdK z8qD|HW_n2YhsAnGK%jG6LqN3gcMv!!4)`4p1jP480Y~Hoh>0&j3g+&&LO~gH&j3(9 z__A}{Lm1N8_?`$~470f1z`DVKLaiYwhpooR!a)fa{(9pJ0zqFe2!w%X_Zd@4R2 zpNBt(ufQAeSMlBW0sIsEG~SAThj+l&99~2)5lX}osf2M`o+~`Jdmi@u!t;ig zmzUTp&14N< zg`d{%Nx#*8Z~2||>+l!(%lwP{P5z7gxB7qNeV_SxR2rO&O9{vnwmbjX5`9U-SfZuceo=Ju`byQJ@)zF+p^^o!{? zwqI?(m--#%32d*4=aNxB; z!GkgfO&PRo(0hZf4i*g_IoL3G+2HpFe@h0DS)`F%K^`L8LkENwggQdkhaL;PBaRZ6 zisy)Th`$W;3`-5G3|kh~6xJR-I9wGzBYa!<=MjR4)QGBx zFz&FlVdi0t!_LMEW3yswVzzxR$$pBDiKpUM$Dc~@PRL7`nXotEdg9PTI`O5%b4j8kRnpT*O-a8crzG2wwY~&S)3|BbX*1K_N&8ivENA39+mZhqDIgoHjcQM zE>72`uTO6sNsgR6a^1+*jL-~iMq|c>%&<&d=BCWcSwpg>WWAd8L$);Amc1+cmO`$0 zLa{%G$jQ%HkaH|IAh$GkRqj`L;yiucw!HTIr2NP8_ZM&rMi(q8IIA3}q?B8f?W0mg zJu&LgXyNFR(W^#ZD2yqz74B2vs!^(?sxQVwj4_YdQv`~XMN5mmERHIkTKx7{-q_-? ztH*w$mZ@i`kCcc?CY5Y0xv9z4EY_SajV^VRHkJ97O)T3|cB?$6yuQ43oMhY+<31VR zZ@gjr-U<8(WfL|{=y)Xekry7hG%;!7{E25Cje4~1(T^VM_n7gqcPIHy(oT9~GIw(M z2y7PxiY=7zVeDbUB6U+wJM{kq3W7J zVOV9jKBZvF+9|iHi>fzQcN)u$J4_y?$)>l=0p@D+AhVI#oXPg{kef z(Y8%?%>Jl-FB1q4Q%4=qj;9=LH6v?Q*ZfjjR{KVsf1SDR=(Lz=&rZ8MJ#YHUk8>WU z9&efvI%D>XwkNWlSpOvUWW|$BGs9-in|Wzg{;XGKd(N(&{plR(occLG&n=(3Z{EOp zv*ulVD*vf%^L^&q=AV6f_|t2j;XGq_=Ge0d&#rv-_XWBIA1{&~su zU%$ICvaw+UZ$r(7Z#I@~Jo0kt%Ud?}+q7`gZ=21VTVGMXa%fA+mMvQcY+bw!-{#nM z_0@^5p7?jpzxQsB-M)TD@Q#JAf!7?bef#?4*U#)6y|d|!)Hk-j8S&-No;DzBli^&iALk-?87i|Jnil zfs0L(n$8~_cktAqv4@Tw9)0-35Ar@ZcqHq{`yZx%_|8Z2kKXz?_2WICB!9B|XwuPL zpC)~}>sa!!-N#do?>&)rVqf!!=J#4MS`M7dIr+gU<*83jk2&3Zru59YvlGv@p3|MX z@|p3o>z~(re&_tGFSuVU_|osohOY*E)%c$w|Jl)+-1=@?PTSE7B^S7mY9W?-kC~+SY#DC}gASO;oNQ_JrhlGg#_a7kk zheulH`$HJB#~Aoip8JiVn}Pr*wVUXNUjGp=&*u>Y568M*{{_h4>p!tbI~6|udp&zS zxA{O|yE@F4P|-H$x+1Z@ac59VNw~d<;>}%p;inBb8eK$EZg3=%TuQs1AO88l5ZE!W(xcmgG?O zrk1%&H)t;W#0`RP40r8g6NEP#5Uid7^i8bMNSu-q9y} oN1y5)efl Date: Mon, 25 Jun 2018 12:39:26 -0700 Subject: [PATCH 7/8] Additional tests around upload of non-ascii file paths, and logic tweaks around the same. --- shotgun_api3/shotgun.py | 17 ++++++++--------- tests/test_api.py | 39 ++++++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index ae34ae65..b9a92bbf 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2252,21 +2252,20 @@ def upload(self, entity_type, entity_id, path, field_name=None, display_name=Non # Basic validations of the file to upload. path = os.path.abspath(os.path.expanduser(path or "")) - # On Windows, if a path is given as a string encoded as something other - # than ascii and it contains non-ascii characters, the os.path calls to - # determine its validity will return a false negative. Likewise, later - # in the upload process, opening a filehandle will fail in the same way. - # To combat this, we'll try to decode the string path into unicode, - # which Windows handles well, and that we have proper support for in - # upload pipeline here. - if not isinstance(path, unicode) and sys.platform == "win32": + # We need to check for string encodings that we aren't going to be able + # to support later in the upload process. If the given path wasn't already + # unicode, we will try to decode it as utf-8, and if that fails then we + # have to raise a sane exception. This will always work for ascii and utf-8 + # encoded strings, but will fail on some others if the string includes non + # ascii characters. + if not isinstance(path, unicode): try: path = path.decode("utf-8") except UnicodeDecodeError: raise ShotgunError( "Could not upload the given file path. It is encoded as " "something other than utf-8 or ascii. To upload this file, " - "it can be decoded into unicode prior to upload: %s" % path + "it can be encoded as utf-8, or given as unicode: %s" % path ) if not os.path.isfile(path): diff --git a/tests/test_api.py b/tests/test_api.py index d67386b5..b84f35dc 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -250,22 +250,31 @@ def test_upload_download(self): tag_list="monkeys, everywhere, send, help" ) - # Make sure that non-utf-8 encoded paths raise on Windows. - if sys.platform == "win32": - u_path = os.path.abspath( - os.path.expanduser( - glob.glob(os.path.join(unicode(this_dir), u'*.shift-jis'))[0] - ) - ) - self.assertRaises( - shotgun_api3.ShotgunError, - self.sg.upload, - "Ticket", - self.ticket['id'], - u_path.encode("shift-jis"), - 'attachments', - tag_list="monkeys, everywhere, send, help" + # Make sure that non-utf-8 encoded paths raise when they can't be + # converted to utf-8. + u_path = os.path.abspath( + os.path.expanduser( + glob.glob(os.path.join(unicode(this_dir), u'*.shift-jis'))[0] ) + ) + self.assertRaises( + shotgun_api3.ShotgunError, + self.sg.upload, + "Ticket", + self.ticket['id'], + u_path.encode("shift-jis"), + 'attachments', + tag_list="monkeys, everywhere, send, help" + ) + + # But it should work in all cases if a unicode string is used. + self.sg.upload( + "Ticket", + self.ticket['id'], + u_path, + 'attachments', + tag_list="monkeys, everywhere, send, help" + ) # cleanup os.remove(file_path) From dc37cc247f651e75b6408dbc449455138a95e64a Mon Sep 17 00:00:00 2001 From: Jeff Beeland Date: Mon, 25 Jun 2018 12:53:58 -0700 Subject: [PATCH 8/8] [minor] Tweaks an error message to be extra clear. --- shotgun_api3/shotgun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index b9a92bbf..bee32129 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -2265,7 +2265,7 @@ def upload(self, entity_type, entity_id, path, field_name=None, display_name=Non raise ShotgunError( "Could not upload the given file path. It is encoded as " "something other than utf-8 or ascii. To upload this file, " - "it can be encoded as utf-8, or given as unicode: %s" % path + "it can be string encoded as utf-8, or given as unicode: %s" % path ) if not os.path.isfile(path):