Skip to content

Commit 93cd1d4

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 6eceed3 commit 93cd1d4

12 files changed

Lines changed: 87 additions & 61 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.
@@ -205,8 +207,7 @@ def param_serialize(
205207
:param _request_auth: set to override the auth_settings for an a single
206208
request; this effectively ignores the authentication
207209
in the spec for a single request.
208-
:return: tuple of form (path, http_method, query_params, header_params,
209-
body, post_params, files)
210+
:return: tuple of (method, url, header_params, body, post_params)
210211
"""
211212

212213
config = self.configuration
@@ -578,21 +579,14 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
578579
elif isinstance(v, bytes):
579580
filedata = v
580581

581-
# Validate file size
582-
if len(filedata) < 4:
583-
raise ValueError(f"File data too small ({len(filedata)} bytes) - minimum 4 bytes required")
584-
if len(filedata) > 2 * 1024 * 1024: # 2MB limit (matches Okta's background image limit)
585-
raise ValueError(f"File data too large ({len(filedata)} bytes) - maximum 2MB allowed")
586-
587582
# Detect file type from magic bytes
588-
# Note: This is client-side validation only. Okta API performs
589-
# comprehensive server-side image validation as the security boundary.
590-
if filedata.startswith(b'\x89PNG\r\n\x1a\n'): # Full PNG signature
583+
# Note: This is client-side detection for correct Content-Type
584+
# assignment. Okta API performs server-side image validation
585+
# as the security boundary.
586+
if filedata.startswith(b'\x89PNG\r\n\x1a\n'):
591587
filename = f"{k}.png"
592588
mimetype = "image/png"
593-
elif filedata.startswith(b'\xFF\xD8\xFF\xE0') or \
594-
filedata.startswith(b'\xFF\xD8\xFF\xE1') or \
595-
filedata.startswith(b'\xFF\xD8\xFF\xDB'): # JPEG variants
589+
elif filedata.startswith(b'\xFF\xD8'): # All JPEG variants start with SOI marker
596590
filename = f"{k}.jpg"
597591
mimetype = "image/jpeg"
598592
elif filedata.startswith(b'GIF87a') or filedata.startswith(b'GIF89a'):
@@ -602,6 +596,7 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
602596
# For unknown types, attempt PIL validation if available
603597
pil_validated = False
604598
try:
599+
from PIL import Image # Lazy import — Pillow is optional
605600
img = Image.open(io.BytesIO(filedata))
606601
img.verify() # Verify it's actually a valid image
607602
format_to_ext = {'PNG': 'png', 'JPEG': 'jpg', 'GIF': 'gif'}
@@ -610,17 +605,24 @@ def files_parameters(self, files: Dict[str, Union[str, bytes]]):
610605
mimetype = f"image/{ext if ext != 'jpg' else 'jpeg'}"
611606
pil_validated = True
612607
except ImportError:
613-
# PIL not available - continue to fallback
614-
pass
608+
logger.warning(
609+
"Could not detect file type from magic bytes and "
610+
"Pillow is not installed for advanced detection. "
611+
"Install it with: pip install pillow"
612+
)
615613
except Exception:
616-
# PIL validation failed - file may not be a valid image
617-
pass
614+
logger.warning(
615+
"File type detection failed — the file may not "
616+
"be a valid image. Okta accepts PNG, JPEG, and "
617+
"GIF formats only."
618+
)
618619

619620
if not pil_validated:
620-
# Fallback: Default to PNG with warning
621-
# Server-side validation will reject if not a valid image
622-
filename = f"{k}.png"
623-
mimetype = "image/png"
621+
# Fallback: default to application/octet-stream for
622+
# unrecognized types; server-side validation will
623+
# reject if the file is not a valid image.
624+
filename = f"{k}.bin"
625+
mimetype = "application/octet-stream"
624626
else:
625627
raise ValueError("Unsupported file value")
626628
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: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ async def create_request(
134134
url: str,
135135
body: dict = None,
136136
headers: dict = None,
137-
form: dict = None,
137+
form=None,
138138
oauth=False,
139139
keep_empty_params=False,
140140
):
@@ -146,9 +146,11 @@ async def create_request(
146146
url (str): URL to send request to
147147
body (dict, optional): Request body. Defaults to None.
148148
headers (dict, optional): Request headers. Defaults to None.
149-
form (dict, optional): Form data. Defaults to None.
149+
form (dict or list, optional): Form data. Dict for url-encoded forms,
150+
list of tuples for multipart file uploads. Defaults to None.
150151
oauth: Should use oauth? Defaults to False.
151-
keep_empty_params: Should request body keep parameters with empty values? Defaults to False.
152+
keep_empty_params: Should request body keep parameters with empty
153+
values? Defaults to False.
152154
153155
Returns:
154156
dict, Exception: Tuple of Dictionary repr of HTTP request and
@@ -164,6 +166,8 @@ async def create_request(
164166

165167
# Build request
166168
# Get predetermined headers and build URL
169+
if headers is None:
170+
headers = {}
167171
headers = {**self._custom_headers, **headers}
168172
headers = {**self._default_headers, **headers}
169173
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

@@ -40,6 +40,8 @@ from {{packageName}}.exceptions.exceptions import (
4040

4141
RequestSerialized = Tuple[str, str, Dict[str, str], Optional[str], List[str]]
4242

43+
logger = logging.getLogger("okta-sdk-python")
44+
4345
class ApiClient:
4446
"""Generic API client for OpenAPI client library builds.
4547

@@ -208,8 +210,7 @@ class ApiClient:
208210
:param _request_auth: set to override the auth_settings for an a single
209211
request; this effectively ignores the authentication
210212
in the spec for a single request.
211-
:return: tuple of form (path, http_method, query_params, header_params,
212-
body, post_params, files)
213+
:return: tuple of (method, url, header_params, body, post_params)
213214
"""
214215

215216
config = self.configuration
@@ -591,21 +592,14 @@ class ApiClient:
591592
elif isinstance(v, bytes):
592593
filedata = v
593594

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

632633
if not pil_validated:
633-
# Fallback: Default to PNG with warning
634-
# Server-side validation will reject if not a valid image
635-
filename = f"{k}.png"
636-
mimetype = "image/png"
634+
# Fallback: default to application/octet-stream for
635+
# unrecognized types; server-side validation will
636+
# reject if the file is not a valid image.
637+
filename = f"{k}.bin"
638+
mimetype = "application/octet-stream"
637639
else:
638640
raise ValueError("Unsupported file value")
639641
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: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,12 @@ class RequestExecutor:
134134
url: str,
135135
body: dict = None,
136136
headers: dict = None,
137-
form: dict = None,
137+
form=None,
138138
oauth=False,
139139
keep_empty_params=False,
140140
):
141+
async def create_request(self, method: str, url: str, body: dict = None,
142+
headers: dict = None, form=None, oauth=False, keep_empty_params=False):
141143
"""
142144
Creates request for request executor's HTTP client.
143145

@@ -147,8 +149,12 @@ class RequestExecutor:
147149
body (dict, optional): Request body. Defaults to None.
148150
headers (dict, optional): Request headers. Defaults to None.
149151
form (dict, optional): Form data. Defaults to None.
152+
headers (dict, optional): Request headers. Defaults to None.
153+
form (dict or list, optional): Form data. Dict for url-encoded forms,
154+
list of tuples for multipart file uploads. Defaults to None.
150155
oauth: Should use oauth? Defaults to False.
151-
keep_empty_params: Should request body keep parameters with empty values? Defaults to False.
156+
keep_empty_params: Should request body keep parameters with empty
157+
values? Defaults to False.
152158

153159
Returns:
154160
dict, Exception: Tuple of Dictionary repr of HTTP request and
@@ -164,6 +170,8 @@ class RequestExecutor:
164170

165171
# Build request
166172
# Get predetermined headers and build URL
173+
if headers is None:
174+
headers = {}
167175
headers = {**self._custom_headers, **headers}
168176
headers = {**self._default_headers, **headers}
169177
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)