Skip to content

Commit 57b3cde

Browse files
committed
fix: Address review feedback for multipart file upload fix
- Make Pillow an optional dependency (lazy import with try/except) - Remove Pillow from mandatory requirements, setup.py, pyproject.toml - Add extras_require so users can `pip install okta[images]` - Add logger.warning() when file type detection falls back - Fix mutable default args in RequestExecutor.create_request() - Fix param_serialize() docstring to match actual return signature - Use case-insensitive Content-Type header lookup for OAuth forms - Simplify JPEG detection to cover all SOI marker variants - Restore issue #131 reference comment for json param handling
1 parent 58e9047 commit 57b3cde

12 files changed

Lines changed: 87 additions & 63 deletions

okta/api_client.py

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@
2323
import datetime
2424
import io
2525
import json
26+
import logging
2627
import mimetypes
2728
import os
2829
import re
2930
import tempfile
3031
from enum import Enum
31-
from PIL import Image
3232
from typing import Tuple, Optional, List, Dict, Union
3333
from urllib.parse import quote
3434

@@ -49,6 +49,8 @@
4949

5050
RequestSerialized = Tuple[str, str, Dict[str, str], Optional[str], List[str]]
5151

52+
logger = logging.getLogger("okta-sdk-python")
53+
5254

5355
class ApiClient:
5456
"""Generic API client for OpenAPI client library builds.
@@ -189,8 +191,7 @@ def param_serialize(
189191
:param _request_auth: set to override the auth_settings for an a single
190192
request; this effectively ignores the authentication
191193
in the spec for a single request.
192-
:return: tuple of form (path, http_method, query_params, header_params,
193-
body, post_params, files)
194+
:return: tuple of (method, url, header_params, body, post_params)
194195
"""
195196

196197
config = self.configuration
@@ -562,21 +563,14 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
562563
elif isinstance(v, bytes):
563564
filedata = v
564565

565-
# Validate file size
566-
if len(filedata) < 4:
567-
raise ValueError(f"File data too small ({len(filedata)} bytes) - minimum 4 bytes required")
568-
if len(filedata) > 2 * 1024 * 1024: # 2MB limit (matches Okta's background image limit)
569-
raise ValueError(f"File data too large ({len(filedata)} bytes) - maximum 2MB allowed")
570-
571566
# Detect file type from magic bytes
572-
# Note: This is client-side validation only. Okta API performs
573-
# comprehensive server-side image validation as the security boundary.
574-
if filedata.startswith(b'\x89PNG\r\n\x1a\n'): # Full PNG signature
567+
# Note: This is client-side detection for correct Content-Type
568+
# assignment. Okta API performs server-side image validation
569+
# as the security boundary.
570+
if filedata.startswith(b'\x89PNG\r\n\x1a\n'):
575571
filename = f"{k}.png"
576572
mimetype = "image/png"
577-
elif filedata.startswith(b'\xFF\xD8\xFF\xE0') or \
578-
filedata.startswith(b'\xFF\xD8\xFF\xE1') or \
579-
filedata.startswith(b'\xFF\xD8\xFF\xDB'): # JPEG variants
573+
elif filedata.startswith(b'\xFF\xD8'): # All JPEG variants start with SOI marker
580574
filename = f"{k}.jpg"
581575
mimetype = "image/jpeg"
582576
elif filedata.startswith(b'GIF87a') or filedata.startswith(b'GIF89a'):
@@ -586,6 +580,7 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
586580
# For unknown types, attempt PIL validation if available
587581
pil_validated = False
588582
try:
583+
from PIL import Image # Lazy import — Pillow is optional
589584
img = Image.open(io.BytesIO(filedata))
590585
img.verify() # Verify it's actually a valid image
591586
format_to_ext = {'PNG': 'png', 'JPEG': 'jpg', 'GIF': 'gif'}
@@ -594,17 +589,24 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
594589
mimetype = f"image/{ext if ext != 'jpg' else 'jpeg'}"
595590
pil_validated = True
596591
except ImportError:
597-
# PIL not available - continue to fallback
598-
pass
592+
logger.warning(
593+
"Could not detect file type from magic bytes and "
594+
"Pillow is not installed for advanced detection. "
595+
"Install it with: pip install pillow"
596+
)
599597
except Exception:
600-
# PIL validation failed - file may not be a valid image
601-
pass
598+
logger.warning(
599+
"File type detection failed — the file may not "
600+
"be a valid image. Okta accepts PNG, JPEG, and "
601+
"GIF formats only."
602+
)
602603

603604
if not pil_validated:
604-
# Fallback: Default to PNG with warning
605-
# Server-side validation will reject if not a valid image
606-
filename = f"{k}.png"
607-
mimetype = "image/png"
605+
# Fallback: default to application/octet-stream for
606+
# unrecognized types; server-side validation will
607+
# reject if the file is not a valid image.
608+
filename = f"{k}.bin"
609+
mimetype = "application/octet-stream"
608610
else:
609611
raise ValueError("Unsupported file value")
610612
params.append(tuple([k, tuple([filename, filedata, mimetype])]))

okta/http_client.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,16 @@ async def send_request(self, request):
143143
# Remove Content-Type header - aiohttp.FormData will set it with boundary
144144
params["headers"] = _remove_content_type_header(request_headers)
145145
else:
146-
# Remove Content-Type header - aiohttp.FormData will set it with boundary
147-
if request_headers.get("Content-Type") == "application/x-www-form-urlencoded":
146+
# Regular form data (e.g., OAuth client_assertion)
147+
# For url-encoded forms, let aiohttp handle encoding
148+
ct = next((v for k, v in request_headers.items()
149+
if k.lower() == "content-type"), None)
150+
if ct == "application/x-www-form-urlencoded":
148151
params["headers"] = _remove_content_type_header(request_headers)
149152
params["data"] = request["form"]
150153
json_data = request.get("json")
154+
# Only include json param when present; empty value causes issues
155+
# (ref: https://github.com/okta/okta-sdk-python/issues/131)
151156
if json_data:
152157
params["json"] = json_data
153158
if self._timeout:

okta/request_executor.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ async def create_request(
121121
method: str,
122122
url: str,
123123
body: dict = None,
124-
headers: dict = {},
125-
form: dict = {},
124+
headers: dict = None,
125+
form=None,
126126
oauth=False,
127127
keep_empty_params=False,
128128
):
@@ -133,9 +133,12 @@ async def create_request(
133133
method (str): HTTP Method to be used
134134
url (str): URL to send request to
135135
body (dict, optional): Request body. Defaults to None.
136-
headers (dict, optional): Request headers. Defaults to {}.
136+
headers (dict, optional): Request headers. Defaults to None.
137+
form (dict or list, optional): Form data. Dict for url-encoded forms,
138+
list of tuples for multipart file uploads. Defaults to None.
137139
oauth: Should use oauth? Defaults to False.
138-
keep_empty_params: Should request body keep parameters with empty values? Defaults to False.
140+
keep_empty_params: Should request body keep parameters with empty
141+
values? Defaults to False.
139142
140143
Returns:
141144
dict, Exception: Tuple of Dictionary repr of HTTP request and
@@ -146,6 +149,8 @@ async def create_request(
146149

147150
# Build request
148151
# Get predetermined headers and build URL
152+
if headers is None:
153+
headers = {}
149154
headers = {**self._custom_headers, **headers}
150155
headers = {**self._default_headers, **headers}
151156
if self._config["client"]["orgUrl"] not in url:

openapi/templates/api_client.mustache

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
import datetime
1212
import io
1313
import json
14+
import logging
1415
import mimetypes
1516
import os
1617
import re
1718
import tempfile
1819
from enum import Enum
19-
from PIL import Image
2020
from typing import Tuple, Optional, List, Dict, Union
2121
from urllib.parse import quote
2222

@@ -46,6 +46,8 @@ from okta.call_info import CallInfo
4646

4747
RequestSerialized = Tuple[str, str, Dict[str, str], Optional[str], List[str]]
4848

49+
logger = logging.getLogger("okta-sdk-python")
50+
4951
class ApiClient:
5052
"""Generic API client for OpenAPI client library builds.
5153

@@ -202,8 +204,7 @@ class ApiClient:
202204
:param _request_auth: set to override the auth_settings for an a single
203205
request; this effectively ignores the authentication
204206
in the spec for a single request.
205-
:return: tuple of form (path, http_method, query_params, header_params,
206-
body, post_params, files)
207+
:return: tuple of (method, url, header_params, body, post_params)
207208
"""
208209

209210
config = self.configuration
@@ -585,21 +586,14 @@ class ApiClient:
585586
elif isinstance(v, bytes):
586587
filedata = v
587588

588-
# Validate file size
589-
if len(filedata) < 4:
590-
raise ValueError(f"File data too small ({len(filedata)} bytes) - minimum 4 bytes required")
591-
if len(filedata) > 2 * 1024 * 1024: # 2MB limit (matches Okta's background image limit)
592-
raise ValueError(f"File data too large ({len(filedata)} bytes) - maximum 2MB allowed")
593-
594589
# Detect file type from magic bytes
595-
# Note: This is client-side validation only. Okta API performs
596-
# comprehensive server-side image validation as the security boundary.
597-
if filedata.startswith(b'\x89PNG\r\n\x1a\n'): # Full PNG signature
590+
# Note: This is client-side detection for correct Content-Type
591+
# assignment. Okta API performs server-side image validation
592+
# as the security boundary.
593+
if filedata.startswith(b'\x89PNG\r\n\x1a\n'):
598594
filename = f"{k}.png"
599595
mimetype = "image/png"
600-
elif filedata.startswith(b'\xFF\xD8\xFF\xE0') or \
601-
filedata.startswith(b'\xFF\xD8\xFF\xE1') or \
602-
filedata.startswith(b'\xFF\xD8\xFF\xDB'): # JPEG variants
596+
elif filedata.startswith(b'\xFF\xD8'): # All JPEG variants start with SOI marker
603597
filename = f"{k}.jpg"
604598
mimetype = "image/jpeg"
605599
elif filedata.startswith(b'GIF87a') or filedata.startswith(b'GIF89a'):
@@ -609,6 +603,7 @@ class ApiClient:
609603
# For unknown types, attempt PIL validation if available
610604
pil_validated = False
611605
try:
606+
from PIL import Image # Lazy import — Pillow is optional
612607
img = Image.open(io.BytesIO(filedata))
613608
img.verify() # Verify it's actually a valid image
614609
format_to_ext = {'PNG': 'png', 'JPEG': 'jpg', 'GIF': 'gif'}
@@ -617,17 +612,24 @@ class ApiClient:
617612
mimetype = f"image/{ext if ext != 'jpg' else 'jpeg'}"
618613
pil_validated = True
619614
except ImportError:
620-
# PIL not available - continue to fallback
621-
pass
615+
logger.warning(
616+
"Could not detect file type from magic bytes and "
617+
"Pillow is not installed for advanced detection. "
618+
"Install it with: pip install pillow"
619+
)
622620
except Exception:
623-
# PIL validation failed - file may not be a valid image
624-
pass
621+
logger.warning(
622+
"File type detection failed — the file may not "
623+
"be a valid image. Okta accepts PNG, JPEG, and "
624+
"GIF formats only."
625+
)
625626

626627
if not pil_validated:
627-
# Fallback: Default to PNG with warning
628-
# Server-side validation will reject if not a valid image
629-
filename = f"{k}.png"
630-
mimetype = "image/png"
628+
# Fallback: default to application/octet-stream for
629+
# unrecognized types; server-side validation will
630+
# reject if the file is not a valid image.
631+
filename = f"{k}.bin"
632+
mimetype = "application/octet-stream"
631633
else:
632634
raise ValueError("Unsupported file value")
633635
params.append(tuple([k, tuple([filename, filedata, mimetype])]))

openapi/templates/okta/http_client.mustache

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,16 @@ class HTTPClient:
131131
# Remove Content-Type header - aiohttp.FormData will set it with boundary
132132
params["headers"] = _remove_content_type_header(request_headers)
133133
else:
134-
# Remove Content-Type header - aiohttp.FormData will set it with boundary
135-
if request_headers.get("Content-Type") == "application/x-www-form-urlencoded":
134+
# Regular form data (e.g., OAuth client_assertion)
135+
# For url-encoded forms, let aiohttp handle encoding
136+
ct = next((v for k, v in request_headers.items()
137+
if k.lower() == "content-type"), None)
138+
if ct == "application/x-www-form-urlencoded":
136139
params["headers"] = _remove_content_type_header(request_headers)
137140
params["data"] = request["form"]
138141
json_data = request.get("json")
142+
# Only include json param when present; empty value causes issues
143+
# (ref: https://github.com/okta/okta-sdk-python/issues/131)
139144
if json_data:
140145
params["json"] = json_data
141146
if self._timeout:

openapi/templates/okta/request_executor.mustache

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,20 @@ class RequestExecutor:
104104
return body
105105

106106
async def create_request(self, method: str, url: str, body: dict = None,
107-
headers: dict = {}, form: dict = {}, oauth=False, keep_empty_params=False):
107+
headers: dict = None, form=None, oauth=False, keep_empty_params=False):
108108
"""
109109
Creates request for request executor's HTTP client.
110110

111111
Args:
112112
method (str): HTTP Method to be used
113113
url (str): URL to send request to
114114
body (dict, optional): Request body. Defaults to None.
115-
headers (dict, optional): Request headers. Defaults to {}.
115+
headers (dict, optional): Request headers. Defaults to None.
116+
form (dict or list, optional): Form data. Dict for url-encoded forms,
117+
list of tuples for multipart file uploads. Defaults to None.
116118
oauth: Should use oauth? Defaults to False.
117-
keep_empty_params: Should request body keep parameters with empty values? Defaults to False.
119+
keep_empty_params: Should request body keep parameters with empty
120+
values? Defaults to False.
118121

119122
Returns:
120123
dict, Exception: Tuple of Dictionary repr of HTTP request and
@@ -127,6 +130,8 @@ class RequestExecutor:
127130

128131
# Build request
129132
# Get predetermined headers and build URL
133+
if headers is None:
134+
headers = {}
130135
headers = {**self._custom_headers, **headers}
131136
headers = {**self._default_headers, **headers}
132137
if self._config["client"]["orgUrl"] not in url:

openapi/templates/pyproject.mustache

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ pycryptodome = ">= 3.9.0"
2727
{{/hasHttpSignatureMethods}}
2828
pydantic = ">=2"
2929
typing-extensions = ">=4.7.1"
30-
pillow = "12.1.1"
3130

3231
[tool.poetry.dev-dependencies]
3332
pytest = ">=7.2.1"

openapi/templates/requirements.mustache

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ aenum==3.1.11
22
aiohttp==3.12.14
33
blinker==1.9.0
44
jwcrypto==1.5.6
5-
pillow==12.1.1
65
pycryptodomex==3.23.0
76
pydantic==2.11.3
87
pydash==8.0.5

openapi/templates/setup.mustache

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ REQUIRES = [
4545
"PyYAML >= 6.0.2",
4646
"requests >= 2.32.3",
4747
"xmltodict >= 0.14.2",
48-
"pillow >= 12.1.1",
4948
]
5049

5150
def get_version():
@@ -78,6 +77,9 @@ setup(
7877
url="https://github.com/okta/okta-sdk-python",
7978
keywords=["OpenAPI", "OpenAPI-Generator", "Okta Admin Management"],
8079
install_requires=REQUIRES,
80+
extras_require={
81+
"images": ["pillow >= 9.0.0"],
82+
},
8183
packages=find_packages(exclude=["test", "tests"]),
8284
include_package_data=True,
8385
license="Apache-2.0",

pyproject.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ urllib3 = ">= 1.25.3"
1616
python-dateutil = ">=2.8.2"
1717
pydantic = ">=2"
1818
typing-extensions = ">=4.7.1"
19-
pillow = "12.1.1"
2019

2120
[tool.poetry.dev-dependencies]
2221
pytest = ">=7.2.1"

0 commit comments

Comments
 (0)