diff --git a/NEWS.md b/NEWS.md index 487a8da..10d6e56 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,10 @@ * Legacy auth functions `clear_token()`, `gmail_auth()`, and `use_secret_file()` have been removed, following the same deprecation timeline as described above. +## Bug fixes + +* Fixed MIME structure for emails with text+HTML bodies and attachments. These messages now correctly use nested `multipart/mixed` (outer) containing `multipart/alternative` (text/HTML), preventing the loss of some of the message parts (#202). + # gmailr 2.0.0 ## Changes around the OAuth client diff --git a/R/gm_mime.R b/R/gm_mime.R index 36fff70..ca31931 100644 --- a/R/gm_mime.R +++ b/R/gm_mime.R @@ -243,14 +243,28 @@ as.character.mime <- function(x, newline = "\r\n", ...) { # encode headers x$header <- lapply(x$header, header_encode) - # if we have both the text part and html part, we have to embed them in a multipart/alternative message - if ( - x$attr$content_type %!=% - "multipart/alternative" && - exists_list(x$parts, TEXT_PART) && - exists_list(x$parts, HTML_PART) - ) { - x$attr$content_type <- "multipart/alternative" + # Check if we need nested structure ((text + HTML) + attachments) + has_both_bodies <- exists_list(x$parts, TEXT_PART) && + exists_list(x$parts, HTML_PART) + # Attachments, if present, always start at index 3 + has_attachments <- length(x$parts) > 2 + + if (has_both_bodies) { + if (has_attachments) { + # Need a nested structure: + # multipart/mixed containing [multipart/alternative [text, html], attachment1, ...] + alternative_part <- gm_mime( + attr = list(content_type = "multipart/alternative"), + parts = list(x$parts[[TEXT_PART]], x$parts[[HTML_PART]]) + ) + + attachment_parts <- x$parts[3:length(x$parts)] + x$parts <- c(list(alternative_part), attachment_parts) + + x$attr$content_type <- "multipart/mixed" + } else { + x$attr$content_type <- "multipart/alternative" + } } # if a multipart message diff --git a/tests/testthat/test-gm_mime.R b/tests/testthat/test-gm_mime.R index e6c62e0..1db4523 100644 --- a/tests/testthat/test-gm_mime.R +++ b/tests/testthat/test-gm_mime.R @@ -1,46 +1,30 @@ test_that("MIME - Basic functions", { - # Create a new Email::Stuffer object msg <- gm_mime() - expect_equal(class(msg), "mime", label = "msg object has correct class") - - expect_true( - length(msg$header) > 0, - label = "Even the default object has headers" - ) + expect_s3_class(msg, "mime") + expect_true(length(msg$header) > 0) rv <- gm_to(msg, "adam@ali.as") - expect_equal( - header_encode(rv$header$To), - "adam@ali.as", - label = "to sets To Header" - ) + expect_equal(header_encode(rv$header$To), "adam@ali.as") rv <- gm_from(msg, "bob@ali.as") - expect_equal( - header_encode(rv$header$From), - "bob@ali.as", - label = "from sets From Header" - ) + expect_equal(header_encode(rv$header$From), "bob@ali.as") rv <- gm_to(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$To), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "to (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) rv <- gm_cc(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$Cc), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "cc (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) rv <- gm_bcc(msg, c("adam@ali.as", "another@ali.as", "bob@ali.as")) expect_equal( header_encode(rv$header$Bcc), - "adam@ali.as, another@ali.as, bob@ali.as", - label = "bcc (multiple) sets To header" + "adam@ali.as, another@ali.as, bob@ali.as" ) }) @@ -68,62 +52,51 @@ test_that("header_encode encodes non-ascii values as base64", { }) test_that("MIME - More Complex", { - msg <- gm_mime() - msg <- gm_from(msg, "Jim Hester") - msg <- gm_to(msg, "james.f.hester@gmail.com") - msg <- gm_subject(msg, "Hello To:!") - msg <- gm_text_body(msg, "I am an email") - - msg1 <- gm_attach_file(msg, test_path("fixtures", "volcano.png")) - + msg <- gm_mime() |> + gm_from("Gargle Testuser ") |> + gm_to("jenny+gmailr-tests@posit.co") |> + gm_subject("Hello To:!") |> + gm_text_body("I am an email") + + # Text body with PNG attachment + msg1 <- msg |> gm_attach_file(test_path("fixtures", "volcano.png")) msg1_chr <- as.character(msg1) - - expect_match(msg1_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg1_chr, "gmail", label = "Email contains to string") - expect_match(msg1_chr, "Hello", label = "Email contains subject string") - expect_match(msg1_chr, "I am an email", label = "Email contains text_body") - expect_match(msg1_chr, "volcano", label = "Email contains file name") - - msg2 <- gm_attach_file( - msg, - test_path("fixtures", "test.ini"), - content_type = "text/plain" - ) - + expect_match(msg1_chr, "Gargle Testuser") + expect_match(msg1_chr, "posit") + expect_match(msg1_chr, "Hello") + expect_match(msg1_chr, "I am an email") + expect_match(msg1_chr, "volcano") + + # Text body with text attachment + msg2 <- msg |> + gm_attach_file( + test_path("fixtures", "test.ini"), + content_type = "text/plain" + ) msg2_chr <- as.character(msg2) - - expect_match(msg2_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg2_chr, "gmail", label = "Email contains to string") - expect_match(msg2_chr, "Hello", label = "Email contains subject string") - expect_match(msg2_chr, "I am an email", label = "Email contains text_body") + expect_match(msg2_chr, "I am an email") expect_match( msg2_chr, - "Content-Type: application/octet-stream; name=test\\.ini", - label = "Email contains attachment Content-Type" - ) - - msg3 <- gm_html_body(msg, "I am an html email
") - msg3 <- gm_attach_file( - msg3, - test_path("fixtures", "test.ini"), - content_type = "application/octet-stream" + "Content-Type: application/octet-stream; name=test\\.ini" ) + # Text + HTML body with attachment + msg3 <- msg |> + gm_html_body("

I am an html email

") |> + gm_attach_file( + test_path("fixtures", "test.ini"), + content_type = "application/octet-stream" + ) msg3_chr <- as.character(msg3) - - expect_match(msg3_chr, "Jim Hester", label = "Email contains from name") - expect_match(msg3_chr, "gmail", label = "Email contains to string") - expect_match(msg3_chr, "Hello", label = "Email contains subject string") - expect_match(msg3_chr, "I am an email", label = "Email contains text_body") + expect_match(msg3_chr, "I am an email") expect_match( msg3_chr, - base64url_encode("I am an html email
"), - label = "Email contains html_body" + base64encode(charToRaw("

I am an html email

")), + fixed = TRUE ) expect_match( msg3_chr, - "Content-Type: application/octet-stream; name=test\\.ini", - label = "Email contains attachment Content-Type" + "Content-Type: application/octet-stream; name=test\\.ini" ) skip_if_no_token() @@ -136,47 +109,107 @@ test_that("MIME - More Complex", { }) test_that("MIME - Alternative emails contain correct parts", { - msg <- gm_mime() - msg <- gm_from(msg, "Jim Hester") - msg <- gm_to(msg, "james.f.hester@gmail.com") - msg <- gm_subject(msg, "Hello To:!") - msg <- gm_text_body(msg, "I am an email") - msg <- gm_html_body(msg, "I am a html email") + msg <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Hello!") |> + gm_text_body("I am an email") |> + gm_html_body("I am a html email") email_chr <- as.character(msg) - expect_match(email_chr, "Jim Hester", label = "Email contains from name") - expect_match(email_chr, "james.f.hester", label = "Email contains to string") - expect_match(email_chr, "Hello", label = "Email contains subject string") + expect_match(email_chr, "test@example\\.com") + expect_match(email_chr, "user@example\\.com") + expect_match(email_chr, "Hello") + expect_match(email_chr, "Content-Type: multipart/alternative") + expect_match(email_chr, "Content-Type: text/plain") + expect_match(email_chr, "Content-Type: text/html") + expect_match(email_chr, quoted_printable_encode("I am an email")) expect_match( email_chr, - "Content-Type: multipart/alternative", - label = "Email content type" - ) - expect_match( - email_chr, - "Content-Type: text/plain", - label = "Email content type" - ) - expect_match( - email_chr, - "Content-Type: text/html", - label = "Email content type" + base64encode(charToRaw("I am a html email")), + fixed = TRUE ) +}) +test_that("MIME - Messages with attachments and alternative bodies", { + # Test 1: text + HTML + attachment should have nested structure + msg1 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Test with attachment") |> + gm_text_body("Plain text version") |> + gm_html_body("HTML version") |> + gm_attach_file(test_path("fixtures", "volcano.png")) + + msg1_chr <- as.character(msg1) + + # Verify outer is multipart/mixed + expect_match(msg1_chr, "Content-Type: multipart/mixed") + # Verify nested multipart/alternative exists + expect_match(msg1_chr, "Content-Type: multipart/alternative") + # Verify all parts present + expect_match(msg1_chr, "Plain text version") expect_match( - email_chr, - quoted_printable_encode("I am an email"), - label = "Email contains text body" - ) + msg1_chr, + base64encode(charToRaw("HTML version")), + fixed = TRUE + ) + expect_match(msg1_chr, "volcano\\.png") + + # Test 2: text + HTML + text attachment + # https://github.com/r-lib/gmailr/issues/202 + msg2 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Test with text attachment") |> + gm_text_body("Email body text") |> + gm_html_body("

Email body HTML

") |> + gm_attach_file(test_path("fixtures", "test.ini")) + + msg2_chr <- as.character(msg2) + + expect_match(msg2_chr, "Content-Type: multipart/mixed") + expect_match(msg2_chr, "Content-Type: multipart/alternative") + expect_match(msg2_chr, "Email body text") expect_match( - email_chr, - base64encode(charToRaw("I am a html email")), - fixed = TRUE, - label = "Email contains html body" + msg2_chr, + base64encode(charToRaw("

Email body HTML

")), + fixed = TRUE ) -}) + expect_match(msg2_chr, "test\\.ini") + + # Test 3: text only + attachment (no HTML) should be flat multipart/mixed + msg3 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Text only with attachment") |> + gm_text_body("Just plain text") |> + gm_attach_file(test_path("fixtures", "volcano.png")) + msg3_chr <- as.character(msg3) + + expect_match(msg3_chr, "Content-Type: multipart/mixed") + # Should NOT have multipart/alternative since there's no HTML + expect_false(grepl("multipart/alternative", msg3_chr)) + + # Test 4: Multiple attachments + msg4 <- gm_mime() |> + gm_from("test@example.com") |> + gm_to("user@example.com") |> + gm_subject("Multiple attachments") |> + gm_text_body("Text body") |> + gm_html_body("HTML body") |> + gm_attach_file(test_path("fixtures", "test.ini")) |> + gm_attach_file(test_path("fixtures", "volcano.png")) + + msg4_chr <- as.character(msg4) + + expect_match(msg4_chr, "Content-Type: multipart/mixed") + expect_match(msg4_chr, "Content-Type: multipart/alternative") + expect_match(msg4_chr, "test\\.ini") + expect_match(msg4_chr, "volcano\\.png") +}) test_that("plain ascii should not be encoded", { expect_match(