0

I am setting up a group of emails, and start by extracting information from a MySql table

<cfoutput>
<cfset headls ='PersonFn,PersonLn,PersonEmail1'>
<cfquery name = "sord" datasource = "whatever">
  select distinct PersonID,#headls# from PersonRepDb        
</cfquery>
</cfoutput>

This produces the correct output. I then loop through the results of the query, sending an email to each person. 

  <cfset sordlen = sord.recordcount>
  <cfloop from = "1" to = "#sordlen#" index = 'j'> 
  <cfmail 
          from     = "#session.user#"  
          to       = "#sord['PersonEmail1'][j]#"          
          password = "#session.password#"
          username = "#session.user#"             
          server   = "localhost"                            
          replyto  = "#txt['replyto']#"
          subject  = "#txt['repsubject']#"               
          type     = "html"   >     

     ...stuff
 </cfmail>
 </cfloop>

When I try to run this program I get an error message: "One of the following attributes must be defined [to, cc, bcc]". Obviously the "to" is there, and if I replace the variable with a specific email like "joan@gmail.com" the error message goes away. So apparently the variable after 'to' is not being decoded.

I tried splitting up the variable sord['PersonEmail1'][j] into the parts before and after the @ 

<cfset preml = GetToken("#sord['PersonEmail1'][j]#",1,'@')>
<cfset posml = GetToken("#sord['PersonEmail1'][j]#",2,'@')>

and then setting up the to as

 to = "#preml#@#posml#"

but that did not help.

Can anyone tell me how to fix this?

Betty Mock
  • 1,373
  • 11
  • 23
  • What version of CF? – Shawn Apr 01 '19 at 22:05
  • 2
    And do you need to use dynamic column names? Any time you have a dynamic SQL statement, you run a risk of injection. If you have multiple columns that you may need, you might be better off just selecting all of the columns you need and filtering them later. – Shawn Apr 01 '19 at 22:07
  • There doesn't appear to be anything wrong with your code. Are your other variables returning what you expect? Also, you don't need a `cfloop`, you can specify a `query="sord"` attribute, and it will step through your query results for you. Then you'd just use `.....to=sord.PersonEmail1.....`. – Shawn Apr 01 '19 at 22:36
  • I would prefer cfoutput query = 'sord' , but the output tag seems to cause problems with the cfmail tag. Do you know that I can use that with a cfmail tag? I have multiple people and each must receive his/her own cfmail tag, because the email has dynamic content. You are right that my variable was not returning what I thought. Things seem to be working now. – Betty Mock Apr 01 '19 at 23:03
  • Is the dynamic content also defined in a query? If so, you can combine the records with a more refined query. If it's not, how do you associate the content with the recipient? – Shawn Apr 01 '19 at 23:07
  • And is `txt` dynamic inside a loop also or will all `replyto` and `subject` be the same? – Shawn Apr 01 '19 at 23:11
  • Be the same for each `cfmail` from the query? – Shawn Apr 01 '19 at 23:19
  • I am curious why you are not simply looping over the query using ... – Scott Stroz Apr 02 '19 at 03:33
  • 2
    So what's the actual value of `#sord['PersonEmail1'][j]#` when the error occurs? Obviously sanitize the name and domain. – SOS Apr 02 '19 at 04:43
  • Add to query: WHERE PersonEmail1 LIKE '%@%'. See if you get a different record count. – Scott Jibben Apr 03 '19 at 00:23
  • Shawn, the dynamic content is defined in lots of preliminary coding. The txt[] is indeed dynamic in the loop -- otherwise why bother with it. I associate the content with the recipient by generating it on each iteration of the loop. – Betty Mock Apr 03 '19 at 01:22
  • Scott, I didn't use – Betty Mock Apr 03 '19 at 01:23
  • Ageax, that turned out to be the problem -- the content of #sord['PersonEmail1'][j] was incorrect, which would have been due to a data entry mistake on the part of the user. Obviously my test data had a problem, which is a good thing, because I now know I have to check for a (at least minimally) properly formed email before trying to use it. – Betty Mock Apr 03 '19 at 01:27
  • @BettyMock The code above doesn't dynamically pick up `txt[]` values. If you are regening them each loop, is it also coming from a database? Can it be combined into a single query? There is a bit of code missing. If you are doing things outside of the `cfmail` on each loop, then you won't be able to use the `query` attribute (unless the `txt[]` values can be derived from the db values). – Shawn Apr 03 '19 at 01:27
  • @BettyMock How are your email addresses derived? Are they a specific set of company-provided emails or are they user-defined-whatever-they-setup emails? Validating email address can get pretty complex. :-S – Shawn Apr 03 '19 at 01:29
  • What version of CF and what database are you using? – Shawn Apr 03 '19 at 01:31
  • Shawn, I've reworked the code to be entirely within the cfmail tag. It seems to be picking up the txt[] values okay. The email addresses are the responsibility of the user -- they sit in a table. It is clear that I'm going to have to do some checking, or the users will be getting unpleasant errors from coldfusion. I can see that validating email addresses can get complex. Ben Nadel did a good experiment on IsValid() and that should help. I'll probably also go with a try/catch. – Betty Mock Apr 04 '19 at 02:27
  • Shawn, I believe I'm in Coldfusion 11 -- I'm on Hostek, so whatever they are using. The database is MySql. – Betty Mock Apr 04 '19 at 02:28
  • @BettyMock If it's still CF11, that's about to go EOL, so you might want to look at upgrading. With what you are trying to do, you'll probably have to use a `cfloop` instead of just the `query` attribute in the `cfmail` tag. I'm not sure how it would be picking up values for `txt[]` since that isn't defined in your loop nor part of your query. I think there's probably a lot of missing code that could help determine what you are trying to do. – Shawn Apr 04 '19 at 21:13
  • Shawn, I am using query loop. As for CF-11, I'm using whatever Hostek has on its site. I'm sure they will upgrade when appropriate. The structure txt[] is defined long before I start the loop, and it is not dependent on the loop variable -- it goes with the user, not the recipient. – Betty Mock Apr 05 '19 at 22:09

1 Answers1

2

This should be all you need to do. If you're trying to make the list of columns from the DB dynamic, that's probably not needed. Just validate that the contents of the email column is a valid email format before sending. 

<cfquery name="sord" datasource="whatever">
    select distinct 
        PersonID,
        PersonFn,
        PersonLn,
        PersonEmail1
    from 
        PersonRepDb        
</cfquery>

<cfloop query="sord">
    <cfif isValid("email", sord.PersonEmail1)>
        <cfmail 
            from     = "#session.user#"
            to       = "#sord.PersonEmail1#"
            password = "#session.password#"
            username = "#session.user#"
            server   = "localhost"
            replyto  = "#txt['replyto']#"
            subject  = "#txt['repsubject']#"
            type     = "html">

        ...stuff
    </cfmail>
</cfloop>
Adrian J. Moreno
  • 14,350
  • 1
  • 37
  • 44
  • What is the benefit of a query loop around the cfmail instead of using the query attribute? Won't the query attribute make one connection to the mail server and then bulk spool the emails? Not sure. – Shawn Apr 02 '19 at 19:57
  • Also, `isValid()` can get wonky, and validate some addresses incorrectly. Especially in older versions. Not sure which version this is. Though it's likely to hit the vast majority of needed addresses in most systems. – Shawn Apr 02 '19 at 20:10
  • 1
    Adding a `where clause` to the query so it returns only valid email addresses would allow you to use the query attribute of the cfmail tag. The specifics depend on the RDBMS, which has not been identified. – Dan Bracuk Apr 02 '19 at 20:23
  • @DanBracuk I would 100% agree with that, and I believe that you shouldn't filter query results in code; it should be done in the query itself. The only exception being if you need to use that base query results in multiple places for multiple things in code. Then, it still depends. :-) – Shawn Apr 02 '19 at 20:42
  • Dan, I'll look into the query attribtue of the cfmail tag. It's so helpful to learn about these things. – Betty Mock Apr 03 '19 at 01:29