Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 86
RE1-T115 POI Bug fixes#371
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| using System; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Net.Http; | ||
| using Resgrid.Model; | ||
| using Resgrid.Model.Providers; | ||
| using SharpKml.Dom; | ||
| @@ -15,38 +16,143 @@ public List<Coordinates> ImportFile(Stream input, bool isKmz) | ||
| { | ||
| var coordinates = new List<Coordinates>(); | ||
| if (input == null) | ||
| return coordinates; | ||
| try | ||
| { | ||
| KmlFile file; | ||
| if (isKmz) | ||
| { | ||
| var kmz = KmzFile.Open(input); | ||
| file = kmz.GetDefaultKmlFile(); | ||
| file = kmz?.GetDefaultKmlFile(); | ||
| } | ||
| else | ||
| file = KmlFile.Load(input); | ||
| Kml kml = file.Root as Kml; | ||
| if (kml != null) | ||
| if (file?.Root is Kml kml) | ||
| { | ||
| foreach (var placemark in kml.Flatten().OfType<Placemark>()) | ||
| { | ||
| var coords = new Coordinates(); | ||
| coords.Name = placemark.Name; | ||
| coords.Latitude = placemark.CalculateBounds().Center.Latitude; | ||
| coords.Longitude = placemark.CalculateBounds().Center.Longitude; | ||
| ExtractCoordinates(kml, coordinates); | ||
| coordinates.Add(coords); | ||
| // Resolve NetworkLinks to external KML/KMZ resources | ||
| var networkLinks = kml.Flatten().OfType<NetworkLink>().ToList(); | ||
| foreach (var networkLink in networkLinks) | ||
| { | ||
| try | ||
| { | ||
| ResolveNetworkLink(networkLink, coordinates); | ||
| } | ||
| catch | ||
| { | ||
| // Skip failed network link resolution | ||
| } | ||
| } | ||
| } | ||
| } | ||
| catch | ||
| catch (Exception ex) | ||
| { | ||
| System.Diagnostics.Debug.WriteLine($"KmlProvider.ImportFile error: {ex.Message}"); | ||
| } | ||
| return coordinates; | ||
| } | ||
| private static void ExtractCoordinates(Kml kml, List<Coordinates> coordinates) | ||
| { | ||
| foreach (var placemark in kml.Flatten().OfType<Placemark>()) | ||
| { | ||
| var coords = new Coordinates(); | ||
| coords.Name = placemark.Name; | ||
| try | ||
| { | ||
| var bounds = placemark.CalculateBounds(); | ||
| if (bounds != null) | ||
| { | ||
| coords.Latitude = bounds.Center.Latitude; | ||
| coords.Longitude = bounds.Center.Longitude; | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| // Skip placemarks that fail bounds calculation | ||
| } | ||
| if (coords.Latitude.HasValue && coords.Longitude.HasValue) | ||
| coordinates.Add(coords); | ||
| } | ||
| } | ||
| private static void ResolveNetworkLink(NetworkLink networkLink, List<Coordinates> coordinates) | ||
| { | ||
| if (networkLink?.Link?.Href == null) | ||
| return; | ||
| var href = networkLink.Link.Href.OriginalString; | ||
| if (string.IsNullOrWhiteSpace(href)) | ||
| return; | ||
| // Step 1: URI-based detection — check AbsolutePath for .kmz/.kml suffix | ||
| bool? isKmzFromUri = null; | ||
| if (Uri.TryCreate(href, UriKind.Absolute, out Uri uri)) | ||
| { | ||
| var path = uri.AbsolutePath; | ||
| if (path.EndsWith(".kmz", StringComparison.OrdinalIgnoreCase)) | ||
| isKmzFromUri = true; | ||
| else if (path.EndsWith(".kml", StringComparison.OrdinalIgnoreCase)) | ||
| isKmzFromUri = false; | ||
| } | ||
| using (var httpClient = new HttpClient { Timeout = System.TimeSpan.FromSeconds(30) }) | ||
| using (var response = httpClient.GetAsync(href).GetAwaiter().GetResult()) | ||
| { | ||
| if (!response.IsSuccessStatusCode) | ||
| return; | ||
Comment on lines
+86
to
+110
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: SSRF risk —
At minimum, validate that 🛡️ Sketch of validation + size cap- var href = networkLink.Link.Href.OriginalString;- if (string.IsNullOrWhiteSpace(href))- return;-- using (var httpClient = new HttpClient { Timeout = System.TimeSpan.FromSeconds(30) })- {- var response = httpClient.GetAsync(href).GetAwaiter().GetResult();- if (!response.IsSuccessStatusCode)- return;+ var href = networkLink.Link.Href.OriginalString;+ if (!IsSafeRemoteUri(href, out var uri))+ return;++ using (var httpClient = new HttpClient { Timeout = System.TimeSpan.FromSeconds(30) })+ {+ var response = httpClient.GetAsync(uri).GetAwaiter().GetResult();+ if (!response.IsSuccessStatusCode)+ return;++ const long MaxBytes = 10 * 1024 * 1024; // 10 MB cap+ if (response.Content.Headers.ContentLength is long len && len > MaxBytes)+ return;Where 🤖 Prompt for AI Agents | ||
| // Step 2: Content-Type detection — fallback when URI is inconclusive | ||
| bool? isKmzFromContentType = null; | ||
| var contentType = response.Content.Headers.ContentType?.MediaType; | ||
| if (string.Equals(contentType, "application/vnd.google-earth.kmz", StringComparison.OrdinalIgnoreCase)) | ||
| isKmzFromContentType = true; | ||
| else if (string.Equals(contentType, "application/vnd.google-earth.kml+xml", StringComparison.OrdinalIgnoreCase)) | ||
| isKmzFromContentType = false; | ||
| // Download into memory so we can peek bytes and re-read for parsing | ||
| var contentBytes = response.Content.ReadAsByteArrayAsync().GetAwaiter().GetResult(); | ||
| // Step 3: ZIP magic-byte detection — final fallback | ||
| bool? isKmzFromMagic = null; | ||
| if (contentBytes.Length >= 4) | ||
| { | ||
| // PK\x03\x04 is the ZIP magic number | ||
| isKmzFromMagic = contentBytes[0] == 0x50 && contentBytes[1] == 0x4B | ||
| && contentBytes[2] == 0x03 && contentBytes[3] == 0x04; | ||
| } | ||
| // Precedence: URI suffix > Content-Type header > magic bytes; default to false | ||
| bool isKmz = isKmzFromUri ?? isKmzFromContentType ?? isKmzFromMagic ?? false; | ||
| using (var ms = new MemoryStream(contentBytes)) | ||
| { | ||
| KmlFile file; | ||
| if (isKmz) | ||
| { | ||
| var kmz = KmzFile.Open(ms); | ||
| file = kmz?.GetDefaultKmlFile(); | ||
| if (file == null) | ||
| return; | ||
| } | ||
| else | ||
| { | ||
| file = KmlFile.Load(ms); | ||
| } | ||
| if (file?.Root is Kml kml) | ||
| { | ||
| ExtractCoordinates(kml, coordinates); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -469,7 +469,8 @@ public async Task<ActionResult<GetMapDataResult>> GetMapDataAndMarkers() | ||
| PoiTypeId = poiType.PoiTypeId, | ||
| Name = poiType.Name, | ||
| Color = poiType.Color, | ||
| ImagePath = poiType.Image, | ||
| ImagePath = null, | ||
| PoiImage = poiType.Image, | ||
Comment on lines
+472
to
+473
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preserve These lines hard-null Suggested fix- ImagePath = null,+ ImagePath = poiType.Image,
PoiImage = poiType.Image,- ImagePath = null,+ ImagePath = poiType.Image,
PoiImage = poiType.Image,- ImagePath = null,+ ImagePath = poiType.Image,
PoiImage = poiType.Image,- ImagePath = null,+ ImagePath = poiType.Image,
PoiImage = poiType.Image,Also applies to: 634-635, 654-655, 670-671 🤖 Prompt for AI Agents | ||
| Marker = poiType.Marker, | ||
| IsDestination = poiType.IsDestination | ||
| }); | ||
| @@ -630,7 +631,8 @@ public static PoiTypeResultData ConvertPoiTypeData(PoiType poiType) | ||
| PoiTypeId = poiType.PoiTypeId, | ||
| Name = poiType.Name, | ||
| Color = poiType.Color, | ||
| ImagePath = poiType.Image, | ||
| ImagePath = null, | ||
| PoiImage = poiType.Image, | ||
| Marker = poiType.Marker, | ||
| IsDestination = poiType.IsDestination | ||
| }; | ||
| @@ -649,7 +651,8 @@ public static PoiResultData ConvertPoiData(Poi poi, PoiType poiType) | ||
| Latitude = poi.Latitude, | ||
| Longitude = poi.Longitude, | ||
| Color = poiType.Color, | ||
| ImagePath = poiType.Image, | ||
| ImagePath = null, | ||
| PoiImage = poiType.Image, | ||
| Marker = poiType.Marker, | ||
| IsDestination = poiType.IsDestination | ||
| }; | ||
| @@ -664,7 +667,8 @@ private static MapMakerInfoData ConvertPoiMapMarker(Poi poi, PoiType poiType) | ||
| Latitude = poi.Latitude, | ||
| Title = GetPoiTitle(poi, poiType), | ||
| InfoWindowContent = GetPoiInfoWindowContent(poi, poiType), | ||
| ImagePath = poiType.Image, | ||
| ImagePath = null, | ||
| PoiImage = poiType.Image, | ||
| Marker = poiType.Marker, | ||
| Color = poiType.Color, | ||
| Type = 4, | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -289,6 +289,8 @@ public async Task<IActionResult> POIs() | ||
| { | ||
| var modal = new POIsView(); | ||
| modal.Types = await _mappingService.GetPOITypesForDepartmentAsync(DepartmentId); | ||
| modal.Message = TempData["ImportPOIsMessage"] as string; | ||
| modal.ErrorMessage = TempData["ImportPOIsError"] as string; | ||
| return View(modal); | ||
| } | ||
| @@ -305,37 +307,41 @@ public async Task<IActionResult> AddPOIType() | ||
| } | ||
| [HttpGet] | ||
| public async Task<IActionResult> ImportPOIs() | ||
| public async Task<IActionResult> ImportPOIs(int poiTypeId) | ||
| { | ||
| var model = new ImportPOIsView(); | ||
| model.TypeId = poiTypeId; | ||
| return View(model); | ||
| } | ||
| [HttpPost] | ||
| [ValidateAntiForgeryToken] | ||
| public async Task<IActionResult> ImportPOIs(ImportPOIsView modal, IFormFile fileToUpload, CancellationToken cancellationToken) | ||
| { | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (fileToUpload != null && fileToUpload.Length > 0) | ||
| if (fileToUpload == null || fileToUpload.Length == 0) | ||
| { | ||
| //Path.GetExtension(file.FileName).ToLower() == "kmz" | ||
| //var extenion = file.FileName.Substring(file.FileName.IndexOf(char.Parse(".")) + 1, file.FileName.Length - file.FileName.IndexOf(char.Parse(".")) - 1); | ||
| var extenion = Path.GetExtension(fileToUpload.FileName).ToLower(); | ||
| if (!String.IsNullOrWhiteSpace(extenion)) | ||
| extenion = extenion.ToLower(); | ||
| ModelState.AddModelError("fileToUpload", "Please select a file to upload."); | ||
| } | ||
| else | ||
| { | ||
| var extension = Path.GetExtension(fileToUpload.FileName).ToLower(); | ||
| if (extenion != ".kml" && extenion != ".kmz") | ||
| ModelState.AddModelError("fileToUpload", string.Format("File type ({0}) is not a KMZ or KML extension to import POIs.", extenion)); | ||
| if (extension != ".kml" && extension != ".kmz") | ||
| ModelState.AddModelError("fileToUpload", string.Format("File type ({0}) is not a KMZ or KML extension to import POIs.", extension)); | ||
| if (fileToUpload.Length > 10000000) | ||
| ModelState.AddModelError("fileToUpload", "Document is too large, must be smaller then 10MB."); | ||
| } | ||
| if (modal.TypeId <= 0) | ||
| ModelState.AddModelError("TypeId", "Please select a POI type before importing."); | ||
| if (ModelState.IsValid) | ||
| { | ||
| var coordinates = _kmlProvider.ImportFile(fileToUpload.OpenReadStream(), Path.GetExtension(fileToUpload.FileName).ToLower() == ".kmz"); | ||
| int importedCount = 0; | ||
| foreach (var coordinate in coordinates) | ||
| { | ||
| var poi = new Poi(); | ||
| @@ -348,9 +354,15 @@ public async Task<IActionResult> ImportPOIs(ImportPOIsView modal, IFormFile file | ||
| poi.Longitude = coordinate.Longitude.Value; | ||
| await _mappingService.SavePOIAsync(poi, cancellationToken); | ||
| importedCount++; | ||
| } | ||
| } | ||
| if (importedCount > 0) | ||
| TempData["ImportPOIsMessage"] = string.Format("Successfully imported {0} POI(s).", importedCount); | ||
| else | ||
| TempData["ImportPOIsError"] = "No valid placemarks with coordinates could be found in the uploaded file."; | ||
| return RedirectToAction("POIs"); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Resgrid/Core
Length of output: 3937
🏁 Script executed:
Repository: Resgrid/Core
Length of output: 1397
🏁 Script executed:
Repository: Resgrid/Core
Length of output: 20251
🏁 Script executed:
Repository: Resgrid/Core
Length of output: 893
Replace
Debug.WriteLinewithResgrid.Framework.Loggingand log caught exceptions instead of silently swallowing them.Per coding guidelines, all logging must use
Resgrid.Framework.Logging.LogException()/LogError()/LogInfo()/LogDebug(). Currently, line 54 usesSystem.Diagnostics.Debug.WriteLine, which bypasses the logging pipeline and won't appear in production logs. Additionally, the empty catch blocks at lines 45–48 and 76–79 silently discard exceptions during network-link resolution and bounds calculation, making failures undiagnosable in production.Proposed change
Add
using Resgrid.Framework.Logging;to the imports, then:🤖 Prompt for AI Agents