Conversation
|
OK, @copilot, I need you to ensure this branch is up to date with master. While you are at it, give it a pass to assess whether things are working as expected. |
* Fixing bugs * Working on release version * Forgot to add the website * Preparing for CRAN re-release * Porting C functions to R * Fixing bug on processing visual attributes and start working on issue #35 * Preparing CRAN release * Last details before resubmitting to CRAN * Version submitted on Feb 12, 2020 * Updating URLs * Read GEXF file without node labels. * Starting up paper * Preparing to submit paper (and bumping version) * Fixing the title * Creating Logo * Adjusting logo [skip ci] * Adding statement of need and fixing DOI openjournals/joss-reviews#3369 * Update paper.md * Adding CODE_OF_CONDUCT.md (#43) * Working on examples and credit (#43) * Moving files around * Adding more examples, a head method * Improving documentation + more examples (#43) * Updating news * Finalizing first review at JOSS * Forgot to add readme figure * Checking actions for website * Update website.yml * Preparing GH Actions * Rename ci.tmp to ci.yml * Updating vignette * Updating README (forgot to add code of conduct properly) * Adding website and CI badges * Finalizing spell example * JOSS Accepted (close #43) * Preparing release with citation included * Fixing links * Adding an example with lesmiserables * Forgot to add index * Forcing copy * Adding figure with gexf-js * Finalizing CRAN release * Adding parameters for rel scale * Create FUNDING.yml * Update README.md * Update README.Rmd * Preparing for CRAN patch * Fixing * Updating version number * Adding markdown file * Addressing CRAN comments * Updating badges * Removing trailing png from badges * Initial plan * Fix htmlwidgets dependency and namespace issues Co-authored-by: gvegayon <893619+gvegayon@users.noreply.github.com> --------- Co-authored-by: George G. Vega Yon <g.vegayon@gmail.com> Co-authored-by: SamGG <samuel.granjeaud@inserm.fr> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: gvegayon <893619+gvegayon@users.noreply.github.com>
gvegayon
left a comment
There was a problem hiding this comment.
@copilot, this PR has too many changes. The idea of the PR was to add the functionality to call plot() within rmarkdown, quarto, or shiny. Let's see what things that are fundamental for that funciontality can be preserved and discard the other changes.
There was a problem hiding this comment.
This file needs to be removed. Also, we need to ensure this file is not added to git again
| #' @details | ||
| #' Currently, the only languages supported are: | ||
| #' \Sexpr{paste(names(rgexf:::gexf_js_languages), " (", rgexf:::gexf_js_languages, ")", sep="", collapse=", ")}. | ||
| #' German (de), English (en), French (fr), Spanish (es), Italian (it), |
There was a problem hiding this comment.
What we were doing before seem smarter, why change it?
* Initial plan
* Fix .gitignore patterns and restore dynamic \Sexpr{} in plot.R docs
Co-authored-by: gvegayon <893619+gvegayon@users.noreply.github.com>
* Remove duplicate .Rproj.user entry from .gitignore
Co-authored-by: gvegayon <893619+gvegayon@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: gvegayon <893619+gvegayon@users.noreply.github.com>
|
Thank you for your contribution @gvegayon 🚀! The pkgdown site preview is ready for review 👉 Download here 👈! |
There was a problem hiding this comment.
Pull request overview
This pull request adds HTMLwidget support to the rgexf package, enabling interactive visualization of GEXF graph files in R using the gexf-js JavaScript library. The implementation allows users to embed graph visualizations in R Markdown documents, Shiny applications, and HTML outputs.
- Adds
gexfjs()function for creating htmlwidget-based graph visualizations - Includes helper functions for Shiny integration (
gexfjsOutput(),renderGexfjs()) - Adds extensive gexf-js JavaScript library files and dependencies
- Includes test/example implementations (
test1()function and playground files)
Reviewed changes
Copilot reviewed 18 out of 53 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| R/plot.R | Adds new htmlwidget functions: gexfjs(), test1() and their Shiny wrappers |
| DESCRIPTION | Adds htmlwidgets package dependency |
| NAMESPACE | Exports new htmlwidget-related functions |
| .gitignore | Updates ignore patterns with proper wildcard syntax |
| inst/htmlwidgets/gexfjs.js | Main JavaScript widget binding for gexf-js |
| inst/htmlwidgets/gexfjs.yaml | Dependency configuration for the htmlwidget |
| inst/htmlwidgets/test1.js | Test widget implementation |
| inst/htmlwidgets/lib/gexfjs/* | Complete gexf-js library files (JS, CSS, images) |
| playground/*.html, *.rmd, *.cpp | Example/test files for development |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| last: "Vega Yon" | ||
| }; | ||
|
|
||
| // Looping and contatenating objects |
There was a problem hiding this comment.
Typo in comment: "contatenating" should be "concatenating".
| function fillValue(id, a, b) { | ||
| var txt = document.getElementById(id).innerHTML; | ||
|
|
||
| var x = txt.indexOf(a) |
There was a problem hiding this comment.
Missing semicolon after variable declaration. While JavaScript allows this, adding it ensures consistency with the coding style used elsewhere in the file (e.g., line 11).
| #' @import htmlwidgets | ||
| #' @export | ||
| gexfjs <- function( | ||
| gexf = "lib/gexf-1/lesmiserables.gexf" , # This is the onlyone that is working... for now |
There was a problem hiding this comment.
Typo in comment: "onlyone" should be "only one" (two words).
| gexf = "lib/gexf-1/lesmiserables.gexf" , # This is the onlyone that is working... for now | |
| gexf = "lib/gexf-1/lesmiserables.gexf" , # This is the only one that is working... for now |
| <html> | ||
| <head> | ||
| <script> | ||
| // A simple3 function to get an element ID |
There was a problem hiding this comment.
Typo in comment: "simple3" should be "simple".
| Hello | ||
| </p> | ||
| <p id="id2"> | ||
| Yuo |
There was a problem hiding this comment.
Typo in text content: "Yuo" should be "You".
|
|
||
| // parse gexf data | ||
| var parser = new DOMParser(); | ||
| var data = parser.parseFromString(x.data, "application/xml"); |
There was a problem hiding this comment.
Unused variable data.
| var data = parser.parseFromString(x.data, "application/xml"); |
|
|
||
| // parse gexf data | ||
| var parser = new DOMParser(); | ||
| var data = parser.parseFromString(x.data, "application/xml"); |
There was a problem hiding this comment.
Unused variable data.
| @@ -0,0 +1,161 @@ | |||
| if (typeof jQuery == 'undefined') { | |||
| document.write(unescape("%3Cscript type='text/javascript' src='js/jquery-2.0.2.min.js'%3E%3C/script%3E")); | |||
There was a problem hiding this comment.
Avoid using functions that evaluate strings as code.
| document.write(unescape("%3Cscript type='text/javascript' src='js/jquery-2.0.2.min.js'%3E%3C/script%3E")); | |
| var script = document.createElement('script'); | |
| script.type = 'text/javascript'; | |
| script.src = 'js/jquery-2.0.2.min.js'; | |
| document.head.appendChild(script); |
| # read the gexf file | ||
| data <- paste(tryCatch(readLines(gexf), error = function(e) "e"), collapse="\n") | ||
|
|
||
| # # create a list that contains the settings | ||
| # settings <- list( | ||
| # drawEdges = drawEdges, | ||
| # drawNodes = drawNodes | ||
| # ) | ||
|
|
||
| # pass the data and settings using 'x' | ||
| x <- list( | ||
| path = gexf, | ||
| data = data#, |
There was a problem hiding this comment.
readLines(gexf) will read whatever path/URL is provided and the result is placed into x$data, which is then sent to the browser via the htmlwidget. In a Shiny context, if an attacker can control gexf, they can cause the server to read arbitrary local files (e.g., /etc/passwd) and exfiltrate the contents to the client. Restrict gexf to a known safe directory (use normalizePath and a base-dir check), reject absolute/parent (..) paths, and avoid echoing raw file contents back to the client (drop x$data or send only parsed/sanitized fields). For example:
base <- system.file("gexf-graphs", package = "rgexf")
path <- normalizePath(gexf, winslash = "/", mustWork = TRUE)
stopifnot(startsWith(path, normalizePath(base, winslash = "/")))
# Prefer not to include raw file content in x$data
x <- list(path = path)| # read the gexf file | |
| data <- paste(tryCatch(readLines(gexf), error = function(e) "e"), collapse="\n") | |
| # # create a list that contains the settings | |
| # settings <- list( | |
| # drawEdges = drawEdges, | |
| # drawNodes = drawNodes | |
| # ) | |
| # pass the data and settings using 'x' | |
| x <- list( | |
| path = gexf, | |
| data = data#, | |
| # Restrict gexf to a known safe directory | |
| base <- system.file("lib/gexf-1", package = "rgexf") | |
| path <- normalizePath(gexf, winslash = "/", mustWork = TRUE) | |
| if (!startsWith(path, normalizePath(base, winslash = "/"))) { | |
| stop("gexf file must be within the package's lib/gexf-1 directory.") | |
| } | |
| # pass only the safe path using 'x' | |
| x <- list( | |
| path = path |
| width = NULL, | ||
| height = NULL | ||
| ) { | ||
|
|
There was a problem hiding this comment.
Using readLines(gexf) without validating the scheme allows remote URLs (e.g., http:// or https://) to be fetched by the server, enabling SSRF. An attacker controlling gexf in a Shiny app could force requests to internal services (e.g., http://169.254.169.254/ or http://localhost:...) and the response would be embedded in the widget. Disallow URL schemes entirely or enforce a strict allowlist (and block loopback/link-local ranges), e.g.:
if (grepl("^[a-zA-Z]+://", gexf)) stop("URLs are not allowed")
# or allowlist only file paths under a safe base dir as shown above| # Disallow URL schemes to prevent SSRF | |
| if (grepl("^[a-zA-Z]+://", gexf)) stop("URLs are not allowed for 'gexf' argument") |
This pull request introduces a new HTML widget integration for interactive GEXF graph visualization using
gexfjs, and adds supporting infrastructure to the package for both R and Shiny usage. The main changes include new functions for widget creation and rendering, registration of these functions for export, and addition of all necessary JavaScript, CSS, and configuration files for the widget.New HTML widget and Shiny integration:
gexfjs,gexfjsOutput, andrenderGexfjsto create and render an interactive GEXF graph widget, including Shiny support. Also added analogous test widget functions (test1,test1Output,renderTest1). (R/plot.R, R/plot.RR226-R301)NAMESPACE, NAMESPACER17-R30)htmlwidgetsas an imported package inDESCRIPTION. (DESCRIPTION, DESCRIPTIONL24-R25)Widget implementation and dependencies:
inst/htmlwidgets/gexfjs.js), widget configuration YAML (inst/htmlwidgets/gexfjs.yaml), and supporting setup, config, and style files. (inst/htmlwidgets/gexfjs.js, [1];inst/htmlwidgets/gexfjs.yaml, [2];inst/htmlwidgets/lib/gexfjs/setup.js, [3];inst/htmlwidgets/lib/gexfjs/config.js, [4];inst/htmlwidgets/lib/gexfjs/styles/gexfjs.css, [5]Supporting project metadata:
inst/htmlwidgets/lib/gexfjs/.project, inst/htmlwidgets/lib/gexfjs/.projectR1-R17)