Repository navigation
feat: player routes - #3
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated /v1/players/* routing module that exposes player profile/texture data plus derived image endpoints (skin/head/cape) and a binary “batch” endpoint, while refactoring Mojang API calls out of lib.rs.
Changes:
- Introduces new player routes: profile, skin, head, cape, and batch under
/v1/players/{identifier}. - Adds shared helpers for UUID resolution, Mojang profile/texture fetching, base64 decoding, and image extraction.
- Refactors Mojang API logic into
src/mojang.rsand updates router + OpenAPI registration.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/players/utils.rs | Shared utilities (UUID resolution, texture decoding, image extraction, binary response builder). |
| src/routes/players/profile.rs | /v1/players/{identifier} profile endpoint with json/raw output and exclude options. |
| src/routes/players/skin.rs | /skin endpoint returning the raw skin PNG. |
| src/routes/players/head.rs | /head endpoint returning an extracted head PNG. |
| src/routes/players/cape.rs | /cape endpoint returning the raw cape PNG when present. |
| src/routes/players/batch.rs | /batch endpoint returning length-prefixed binary parts (profile/texture/head/skin/cape). |
| src/routes/players.rs | Players module exports and submodule wiring. |
| src/routes/mod.rs | Registers new /v1/players/* routes in the Worker router. |
| src/mojang.rs | Mojang API fetch helpers (UUID + profile). |
| src/models.rs | Adds decoded-textures structs for interpreting Mojang texture payloads. |
| src/lib.rs | Removes old single route and registers new routes + OpenAPI paths. |
| me.sh | Updates local curl helper to new /v1/players/... path. |
| Cargo.toml | Adds serde_json and image dependencies. |
| Cargo.lock | Locks new transitive dependencies for image + serde_json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let exclude_param = req_url | ||
| .query_pairs() | ||
| .find(|(k, _)| k == "exclude") | ||
| .map(|(_, v)| v.to_string()) | ||
| .unwrap_or_default(); | ||
|
|
||
| let exclude_uuid = exclude_param.contains("uuid"); | ||
| let exclude_username = exclude_param.contains("username"); | ||
|
|
There was a problem hiding this comment.
exclude_uuid / exclude_username are computed via exclude_param.contains("uuid") and contains("username"), which does substring matching (e.g., exclude=guid would incorrectly exclude uuid). Parse exclude as comma-separated tokens and check exact field names instead (case/whitespace trimmed).
| pub fn add_string(mut self, s: &str) -> Self { | ||
| self.buffer.put_u16(s.len() as u16); | ||
| self.buffer.put_slice(s.as_bytes()); | ||
| self | ||
| } | ||
|
|
||
| pub fn add_bytes(mut self, b: &[u8]) -> Self { | ||
| self.buffer.put_u16(b.len() as u16); | ||
| self.buffer.put_slice(b); | ||
| self |
There was a problem hiding this comment.
BatchBuilder::add_string / add_bytes cast the part length to u16 (as u16) without validation. If a part ever exceeds 65535 bytes, the length will wrap/truncate and make the binary response unparsable. Consider validating len <= u16::MAX and returning a structured error (or switching to u32 length prefixes).
| let mut head = img.crop_imm(8, 8, 8, 8); | ||
| let hat = img.crop_imm(40, 8, 8, 8); | ||
|
|
||
| image::imageops::overlay(&mut head, &hat, 0, 0); | ||
|
|
There was a problem hiding this comment.
crop_imm(8, 8, 8, 8) / crop_imm(40, 8, 8, 8) will panic if the decoded PNG is smaller than expected. Since the image bytes come from the network, add a dimension check (e.g., ensure width/height are at least 48x16) and return a json_error instead of risking a worker crash.
| let mut head = img.crop_imm(8, 8, 8, 8); | ||
| let hat = img.crop_imm(40, 8, 8, 8); | ||
|
|
||
| image::imageops::overlay(&mut head, &hat, 0, 0); | ||
|
|
There was a problem hiding this comment.
Same as extract_head_png: crop_imm can panic on unexpectedly small/invalid skin images. Add explicit bounds checking before cropping and return an error response on invalid dimensions to avoid panicking in production.
| let mut res = Fetch::Url(url.parse()?).send().await?; | ||
|
|
||
| if res.status_code() == 204 || res.status_code() == 404 { | ||
| return Ok(Err(json_error("player not found", 404)?)); | ||
| } | ||
|
|
||
| match res.json::<MojangUuidResponse>().await { | ||
| Ok(data) => Ok(Ok(data.id)), | ||
| Err(_) => Ok(Err(json_error( | ||
| "failed to parse mojang uuid response", | ||
| 502, | ||
| )?)), | ||
| } |
There was a problem hiding this comment.
fetch_mojang_uuid only treats 204/404 as errors and otherwise always attempts to parse JSON. For non-200 statuses like 429/500, this will likely return a misleading "failed to parse mojang uuid response" instead of a clear upstream error. Consider checking status_code() != 200 (after the 204/404 handling) and returning a consistent 502 response.
No description provided.